Bridge contracts: implement require(cond, error) pattern and update to solc 0.8.27

[andygolay]

Summary

• Addresses [Bridge] [Solidity] Use require for input validation rather than if-revert pattern #827
• Categories: bridge , contracts

Changelog

Update AtomicBridgeInitiatorMOVE.sol and AtomicBridgeCounterparty.sol` to use the require -> custom error pattern.

Testing

Solidity tests pass with forge test --fork-url https://ethereum-sepolia-rpc.publicnode.com -vv in protocol-units/bridge/contracts

Outstanding issues

[franck44]

The scope of the issue is to fix input validation, and use the requires pattern for input validation.

Some of the proposed changes impact code that is not input validation and should not be changed to use requires.

[andygolay]

The scope of the issue is to fix input validation, and use the requires pattern for input validation.

Some of the proposed changes impact code that is not input validation and should not be changed to use requires.

Thank you @franck44, I made the requested changes.

[franck44]

Thanks @andygolay !

[Primata]

@franck44 I believe that if it's not a conditional logic flow, require should be used for consistency.
I think @andygolay could go back to using require on transfer checks.

[andygolay]

@franck44 I believe that if it's not a conditional logic flow, require should be used for consistency. I think @andygolay could go back to using require on transfer checks.

Whatever you guys decide is okay with me; I did look it up and it seems like require is technically fine to use throughout, but it's conventionally used for input values. I wonder if that's because require used to be less gas efficient, so it was used sparingly? And maybe now conventions can shift to using require in general? It does strike me as more normalized syntax to use require for all checks that could revert with a custom error.

The first commit is with all require, the second is with require on input values only. I'll defer to the team as I don't have a strong opinion about it.

[franck44]

@0xPrimata

I believe that if it's not a conditional logic flow, require should be used for consistency.
I think @andygolay could go back to using require on transfer checks.

No, that's not best practice. I provided lots of documentation in issue 827, so we are going to follow best practice.

requires to validate inputs, and reverts for more complex errors.

We can merge this PR and close the issue.

[franck44]

@andygolay

I forgot to mention that there is another reason why requires are used for input validation but not for other revert conditions: requires act as pre-conditions i.e. conditions under which the function can be called. The requires are used by SMTChecker in the Solidity compiler as specifications to verify some properties of the functions.