Basic Client

[ChiTimesChi]

Description

This PR introduces BasicClient, which is the most minimal abstraction of a message sender/recipient. It also adjusts existing Client contract, by making it inherit from BasicClient.

Basic Client

Assumptions

• App is deployed on a few chains, with a single deployment per chain.
• App can only send/receive messages to its deployments on other chains.
• App is enforcing its optimistic period itself.

Security checks

BasicClient features a function to handle incoming messages:

    function _handleUnsafe(
        uint32 _origin,
        uint32 _nonce,
        uint256 _rootSubmittedAt,
        bytes memory _message
    ) internal virtual;

Following statements have been checked prior to _handleUnsafe():

• Message was indeed passed by Destination contract.
• Message sender is app deployment on origin chain

No checks are done for the merkle root timestamp, this allows app to have a flexible optimistic period for different type of messages. Function is named _handleUnsafe to keep dev's attention to the fact that optimistic period needs to be enforced.

Client

Assumptions

• App is deployed on a few chains, with a single deployment per chain.
• App can only send/receive messages to its deployments on other chains.
• App has the same optimistic period on all chains (which is enforced in Client).

Security checks

Client features a function to handle incoming messages:

    function _handle(
        uint32 _origin,
        uint32 _nonce,
        bytes memory _message
    ) internal virtual;

Following statements have been checked to _handle():

• Message was indeed passed by Destination contract.
• Message sender is app deployment on origin chain
• Optimistic period for merkle root (used for proving this message) has passed

[codecov]

Codecov Report

Merging #162 (9dda8ec) into master (a6b87ca) will increase coverage by 0.24027%.
The diff coverage is 100.00000%.

@@                 Coverage Diff                 @@
##              master        #162         +/-   ##
===================================================
+ Coverage   51.81590%   52.05617%   +0.24027%     
===================================================
  Files            141         143          +2     
  Lines           5975        5982          +7     
  Branches          76          77          +1     
===================================================
+ Hits            3096        3114         +18     
+ Misses          2581        2571         -10     
+ Partials         298         297          -1     

Impacted Files	Coverage Δ
...ckages/contracts/contracts/system/SystemRouter.sol	64.70588% <ø> (ø)
.../contracts/test/harnesses/SynapseClientHarness.sol	50.00000% <ø> (ø)
...test/harnesses/SynapseClientUpgradeableHarness.sol	33.33333% <ø> (ø)
...ackages/contracts/contracts/client/BasicClient.sol	100.00000% <100.00000%> (ø)
packages/contracts/contracts/client/Client.sol	100.00000% <100.00000%> (ø)
...ackages/contracts/test/harnesses/ClientHarness.sol	100.00000% <100.00000%> (ø)
ethergo/signer/signer/kmssigner/signing.go	42.39130% <0.00000%> (+3.26087%)	⬆️
agents/indexer/domain.go	67.27273% <0.00000%> (+14.54545%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[aureliusbtc]

should this be enforced to be immutable at the abstract level? or should we allow the inheritor to decide if they want to have the customization ability post-facto.

[ChiTimesChi]

Origin and Destination are supposed to have a permanent address on each chain, as they are never "unenrolled". So I thinking making them immutable on the abstract level is fine.

Ideally deployed on the same address on all chains through restricted create2 factory (just as other "system" contracts).

Restricted to prevent contract address squatting on deployed chains.

[ChiTimesChi]

Once the "create2 factory" is up, could allocate some resources for mining salts resulting in a pretty address. A-la
https://etherscan.io/address/0x11111254369792b2ca5d084ab5eea397ca8fa48b
https://etherscan.io/address/0xdef171fe48cf0115b1d80b88dc8eab59176fee57
https://etherscan.io/address/0x00000000003b3cc22af3ae1eac0440bcee416b40

[aureliusbtc]

same q