Allow the agent and channel to not exist at first

[claravanstaden]

Checking if the agent and channel exists creates a chicken/egg condition where the agent and channel creation cannot be relayed for the first time.

[yrong]

Checking if the agent and channel exists creates a chicken/egg condition where the agent and channel creation cannot be relayed for the first time.

Hey Clara. Not sure if I fully understand the context. Maybe miss something important you and team discussed?

I would assume the agent/channel of bridgeHub is an built-in config when deploy the contracts. For other sibling chains they need to customize the runtime to allow sending xcm to create agent/channel through the channel of bridgeHub(as we do for the template chain). After that they send other messages through their own channels.

So there is no problem but correct me if I'm wrong.

[claravanstaden]

Checking if the agent and channel exists creates a chicken/egg condition where the agent and channel creation cannot be relayed for the first time.

Hey Clara. Not sure if I fully understand the context. Maybe miss something important you and team discussed?

I would assume the agent/channel of bridgeHub is an built-in config when deploy the contracts. For other sibling chains they need to customize the runtime to allow sending xcm to create agent/channel through the channel of bridgeHub(as we do for the template chain). After that they send other messages through their own channels.

So there is no problem but correct me if I'm wrong.

Hey @yrong! If you run start-services.sh, you'll see that the template parachain execution and parachain relayer keeps restarting. If you run the create agent test, you'll see the create agent message is sent to the outbound queue but it doesn't get relayed. So perhaps this is another bug - the bridge hub relayer isn't picking up the message as it should.

[yrong]

you'll see the create agent message is sent to the outbound queue but it doesn't get relayed.

Just tested seems create_agent and create_channel work as expected.

cargo test --test create_agent
...
Event found at ethereum block 278
test create_agent ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 87.78s
...

cargo test --test create_channel
...
test create_channel ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 126.06s

you'll see that the template parachain execution and parachain relayer keeps restarting.

Yes, it's as expected and the log will be fine after we create agent/channel for template parachain.

[claravanstaden]

Let me retest. 😄

[vgeddes]

With solidity, null values are implicitly converted to zero values, so can drop the null check

Suggested change

	// The agent will not exist at first and needs to be created.
	if (ch.agent == address(0)) {
	return (0, 0);
	}

[vgeddes]

I would rather document the behavior in a more generic manner in a doc comment above the function. For example:

A zero address is returned if no matching agent contract is found

This will also help to convey to auditors that this an explicit design choice on our part.

[alistair-singh]

I am not sure if we need to make this change in agentOf to stop the relayers from restarting. We should only need this for channelNoncesOf. Another thing we could do is not change the contracts at all and change the relayers to ignore the specific error, log it out and return default (0, 0) for nonces. The reason for this is that I think these calls returning errors has better UX for people calling them from Etherscan UI or Tenderly.

[yrong]

Agree. I would assume the restarting behavior as expected and not worthwhile to change contract just for that.

Another option is to revamp the e2e script a bit to first create agent/channel before starting the template relayer. To not block the e2e setup too long we can move these to a seperate script. But anyway, it's just a nice to have improvement and not essential.

[vgeddes]

Another thing we could do is not change the contracts at all and change the relayers to ignore the specific error, log it out and return default (0, 0) for nonces

Yes, I like this idea. It means we can leave the contracts alone. 👍

Otherwise we'd have to update all four getters in this file to stop using _ensure*.

[claravanstaden]

I have confirmed the create agent and create channel tests pass, thanks! I think it might have been related to some of the changes I have in-flight on #972.