ZIL-5483: Bridge - ChainId support for multichain calls + others

[WuBruno]

Multiple improvements to the bridge:

• ChainId Support
• Better Error Handling
• Gas Optimisations

[DrZoltanFazekas]

Looks good to me, the updated PR fixed the vulnerability caused by signing only the source chain id. Please take a look at the 3 minor comments I left.

[theo-zil]

Generally looks good to me!

One question: is there going to be any way to validate the chain id in relay()? I.e. to verify that a bridge exists, somehow? Or is the idea that you can call relay() with any chain ID, and the validators will just ignore it if it's not a chain that you're bridged to?

Regarding cross-shard bridging, the interface will work. The Bridged interface can effectively work as-is, just will have to call a precompile for the cross-shard call instead of the _relayer - this can be determined with the chain id, as shard IDs will effectively have a namespace (they will have very large chain IDs, in a very specific range). The dispatched() and queried() functions as I understand it are meant to be used from the other side of the bridge and it's what the physical relayer will invoke on the remote chain, right? For cross-shard these aren't going to be meaningful (since we won't have a twin architecture) so should likely revert if the call is from a shard. Alternatively maybe Bridged can be split into two interfaces, one for outgoing links (containing relay()) and one for incoming ones?

Cross-shard contracts who don't care about cross-chain might in theory find it more convenient to call the bridging precompile directly rather than inheriting an interface. We can provide an identical signature to Relayer.relay() in the precompile, I think, so contracts wanting to directly send a cross-shard message can just make the same call except to the known precompile addess.
Alternatively the logic to check whether the target is a shard (so invoke a precompile) or chain (so continue with the relayer logic) could be placed inside the Relayer, but this will add another layer of indirection (and thus delay and cost) and I don't think it's worthwhile.

[WuBruno]

Generally looks good to me!

One question: is there going to be any way to validate the chain id in relay()? I.e. to verify that a bridge exists, somehow? Or is the idea that you can call relay() with any chain ID, and the validators will just ignore it if it's not a chain that you're bridged to?

Regarding cross-shard bridging, the interface will work. The Bridged interface can effectively work as-is, just will have to call a precompile for the cross-shard call instead of the _relayer - this can be determined with the chain id, as shard IDs will effectively have a namespace (they will have very large chain IDs, in a very specific range). The dispatched() and queried() functions as I understand it are meant to be used from the other side of the bridge and it's what the physical relayer will invoke on the remote chain, right? For cross-shard these aren't going to be meaningful (since we won't have a twin architecture) so should likely revert if the call is from a shard. Alternatively maybe Bridged can be split into two interfaces, one for outgoing links (containing relay()) and one for incoming ones?

Cross-shard contracts who don't care about cross-chain might in theory find it more convenient to call the bridging precompile directly rather than inheriting an interface. We can provide an identical signature to Relayer.relay() in the precompile, I think, so contracts wanting to directly send a cross-shard message can just make the same call except to the known precompile addess. Alternatively the logic to check whether the target is a shard (so invoke a precompile) or chain (so continue with the relayer logic) could be placed inside the Relayer, but this will add another layer of indirection (and thus delay and cost) and I don't think it's worthwhile.

dispatched and queried are indeed is what is invoked on the target chain.

I really like the idea of splitting Bridged into 2 interfaces. I think it makes it easier to understand and compose, especially if some contracts may not necessarily be twins. I am also considering removing the twin construct.

I do agree the check for valid chain/shard id is not necessarily. We can assume the dapp/user/developer to already take care of this.