From 6dfb9507d7c6cfaf7790279fcab52c569e50e3c9 Mon Sep 17 00:00:00 2001 From: Pascal Marco Caversaccio Date: Fri, 29 Apr 2022 17:39:43 +0200 Subject: [PATCH 1/4] feat: refactor the custom errors Signed-off-by: Pascal Marco Caversaccio --- contracts/crosschain/errors.sol | 17 +++++++++++++++-- test/crosschain/CrossChainEnabled.test.js | 4 ++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/contracts/crosschain/errors.sol b/contracts/crosschain/errors.sol index 004460e970b..31c01c23329 100644 --- a/contracts/crosschain/errors.sol +++ b/contracts/crosschain/errors.sol @@ -3,5 +3,18 @@ pragma solidity ^0.8.4; -error NotCrossChainCall(); -error InvalidCrossChainSender(address actual, address expected); +/** + * @dev Error that occurs when a function call is not the result of + * a cross-chain message. + * @param emitter The contract that emits the error. + */ +error NotCrossChainCall(address emitter); + +/** + * @dev Error that occurs when a function call is not the result of + * a cross-chain execution initiated by `account`. + * @param emitter The contract that emits the error. + * @param actual The actual address that initiated the cross-chain execution. + * @param expected The expected address that is allowed to initiate the cross-chain execution. + */ +error InvalidCrossChainSender(address emitter, address actual, address expected); diff --git a/test/crosschain/CrossChainEnabled.test.js b/test/crosschain/CrossChainEnabled.test.js index 6aeb4db7d99..28b9b0d726d 100644 --- a/test/crosschain/CrossChainEnabled.test.js +++ b/test/crosschain/CrossChainEnabled.test.js @@ -15,14 +15,14 @@ function shouldBehaveLikeReceiver (sender = randomAddress()) { it('should reject same-chain calls', async function () { await expectRevertCustomError( this.receiver.crossChainRestricted(), - 'NotCrossChainCall()', + `NotCrossChainCall("${this.receiver.address}")`, ); }); it('should restrict to cross-chain call from a invalid sender', async function () { await expectRevertCustomError( this.bridge.call(sender, this.receiver, 'crossChainOwnerRestricted()'), - `InvalidCrossChainSender("${sender}", "${await this.receiver.owner()}")`, + `InvalidCrossChainSender("${this.receiver.address}", "${sender}", "${await this.receiver.owner()}")`, ); }); From 924a18f962f7ee086f765ceca92a75039a417097 Mon Sep 17 00:00:00 2001 From: Pascal Marco Caversaccio Date: Fri, 29 Apr 2022 17:42:14 +0200 Subject: [PATCH 2/4] feat: refactor the custom errors - v2 Signed-off-by: Pascal Marco Caversaccio --- CHANGELOG.md | 1 + contracts/crosschain/CrossChainEnabled.sol | 4 ++-- contracts/crosschain/amb/LibAMB.sol | 2 +- contracts/crosschain/arbitrum/LibArbitrumL1.sol | 2 +- contracts/crosschain/arbitrum/LibArbitrumL2.sol | 2 +- contracts/crosschain/optimism/LibOptimism.sol | 2 +- .../crosschain/polygon/CrossChainEnabledPolygonChild.sol | 2 +- contracts/mocks/crosschain/bridges.sol | 4 ++-- 8 files changed, 10 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 263f869ff81..9951f8ca484 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * `Clones`: optimize clone creation ([#3329](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3329)) * `TimelockController`: Migrate `_call` to `_execute` and allow inheritance and overriding similar to `Governor`. ([#3317](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3317)) * `CrossChainEnabledPolygonChild`: replace the `require` statement with the custom error `NotCrossChainCall`. ([#3380](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3380)) + * `crosschain`: refactor the custom errors, which now start with the address of the emitter. ([#TBD]()) ## 4.6.0 (2022-04-26) diff --git a/contracts/crosschain/CrossChainEnabled.sol b/contracts/crosschain/CrossChainEnabled.sol index 4c9b9e5c543..c9bde9835ad 100644 --- a/contracts/crosschain/CrossChainEnabled.sol +++ b/contracts/crosschain/CrossChainEnabled.sol @@ -23,7 +23,7 @@ abstract contract CrossChainEnabled { * cross-chain execution. */ modifier onlyCrossChain() { - if (!_isCrossChain()) revert NotCrossChainCall(); + if (!_isCrossChain()) revert NotCrossChainCall(address(this)); _; } @@ -33,7 +33,7 @@ abstract contract CrossChainEnabled { */ modifier onlyCrossChainSender(address expected) { address actual = _crossChainSender(); - if (expected != actual) revert InvalidCrossChainSender(actual, expected); + if (expected != actual) revert InvalidCrossChainSender(address(this), actual, expected); _; } diff --git a/contracts/crosschain/amb/LibAMB.sol b/contracts/crosschain/amb/LibAMB.sol index bd1f4907fd1..c53dfa30d17 100644 --- a/contracts/crosschain/amb/LibAMB.sol +++ b/contracts/crosschain/amb/LibAMB.sol @@ -29,7 +29,7 @@ library LibAMB { * function call is not the result of a cross-chain message. */ function crossChainSender(address bridge) internal view returns (address) { - if (!isCrossChain(bridge)) revert NotCrossChainCall(); + if (!isCrossChain(bridge)) revert NotCrossChainCall(address(this)); return AMB_Bridge(bridge).messageSender(); } } diff --git a/contracts/crosschain/arbitrum/LibArbitrumL1.sol b/contracts/crosschain/arbitrum/LibArbitrumL1.sol index 586888c8b0f..ae3331bdc3b 100644 --- a/contracts/crosschain/arbitrum/LibArbitrumL1.sol +++ b/contracts/crosschain/arbitrum/LibArbitrumL1.sol @@ -33,7 +33,7 @@ library LibArbitrumL1 { * function call is not the result of a cross-chain message. */ function crossChainSender(address bridge) internal view returns (address) { - if (!isCrossChain(bridge)) revert NotCrossChainCall(); + if (!isCrossChain(bridge)) revert NotCrossChainCall(address(this)); address sender = ArbitrumL1_Outbox(ArbitrumL1_Bridge(bridge).activeOutbox()).l2ToL1Sender(); require(sender != address(0), "LibArbitrumL1: system messages without sender"); diff --git a/contracts/crosschain/arbitrum/LibArbitrumL2.sol b/contracts/crosschain/arbitrum/LibArbitrumL2.sol index 54fdb06ff7d..c4aabdcc769 100644 --- a/contracts/crosschain/arbitrum/LibArbitrumL2.sol +++ b/contracts/crosschain/arbitrum/LibArbitrumL2.sol @@ -33,7 +33,7 @@ library LibArbitrumL2 { * function call is not the result of a cross-chain message. */ function crossChainSender(address arbsys) internal view returns (address) { - if (!isCrossChain(arbsys)) revert NotCrossChainCall(); + if (!isCrossChain(arbsys)) revert NotCrossChainCall(address(this)); return ArbitrumL2_Bridge(arbsys).wasMyCallersAddressAliased() diff --git a/contracts/crosschain/optimism/LibOptimism.sol b/contracts/crosschain/optimism/LibOptimism.sol index f84fd126f53..ca615e3d381 100644 --- a/contracts/crosschain/optimism/LibOptimism.sol +++ b/contracts/crosschain/optimism/LibOptimism.sol @@ -29,7 +29,7 @@ library LibOptimism { * function call is not the result of a cross-chain message. */ function crossChainSender(address messenger) internal view returns (address) { - if (!isCrossChain(messenger)) revert NotCrossChainCall(); + if (!isCrossChain(messenger)) revert NotCrossChainCall(address(this)); return Optimism_Bridge(messenger).xDomainMessageSender(); } diff --git a/contracts/crosschain/polygon/CrossChainEnabledPolygonChild.sol b/contracts/crosschain/polygon/CrossChainEnabledPolygonChild.sol index 15da835014b..0d15985f223 100644 --- a/contracts/crosschain/polygon/CrossChainEnabledPolygonChild.sol +++ b/contracts/crosschain/polygon/CrossChainEnabledPolygonChild.sol @@ -63,7 +63,7 @@ abstract contract CrossChainEnabledPolygonChild is IFxMessageProcessor, CrossCha address rootMessageSender, bytes calldata data ) external override nonReentrant { - if (!_isCrossChain()) revert NotCrossChainCall(); + if (!_isCrossChain()) revert NotCrossChainCall(address(this)); _sender = rootMessageSender; Address.functionDelegateCall(address(this), data, "cross-chain execution failed"); diff --git a/contracts/mocks/crosschain/bridges.sol b/contracts/mocks/crosschain/bridges.sol index 6a2b974702e..185b78ac0cf 100644 --- a/contracts/mocks/crosschain/bridges.sol +++ b/contracts/mocks/crosschain/bridges.sol @@ -7,8 +7,8 @@ import "../../vendor/polygon/IFxMessageProcessor.sol"; abstract contract BaseRelayMock { // needed to parse custom errors - error NotCrossChainCall(); - error InvalidCrossChainSender(address sender, address expected); + error NotCrossChainCall(address emitter); + error InvalidCrossChainSender(address emitter, address actual, address expected); address internal _currentSender; From 151bf8bee78b308f65ff4227edf23ad4bc657c4b Mon Sep 17 00:00:00 2001 From: Pascal Marco Caversaccio Date: Fri, 29 Apr 2022 17:51:29 +0200 Subject: [PATCH 3/4] rewording Signed-off-by: Pascal Marco Caversaccio --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9951f8ca484..6d2f1724501 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ * `Clones`: optimize clone creation ([#3329](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3329)) * `TimelockController`: Migrate `_call` to `_execute` and allow inheritance and overriding similar to `Governor`. ([#3317](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3317)) * `CrossChainEnabledPolygonChild`: replace the `require` statement with the custom error `NotCrossChainCall`. ([#3380](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3380)) - * `crosschain`: refactor the custom errors, which now start with the address of the emitter. ([#TBD]()) + * `crosschain`: refactor the custom error parameters, which now start with the address of the emitter. ([#TBD]()) ## 4.6.0 (2022-04-26) From 5c625673552e741ba7f0845d458abdcffe725e09 Mon Sep 17 00:00:00 2001 From: Pascal Marco Caversaccio Date: Fri, 29 Apr 2022 17:56:40 +0200 Subject: [PATCH 4/4] update changelog with PR link Signed-off-by: Pascal Marco Caversaccio --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d2f1724501..dd93a79b05c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ * `Clones`: optimize clone creation ([#3329](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3329)) * `TimelockController`: Migrate `_call` to `_execute` and allow inheritance and overriding similar to `Governor`. ([#3317](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3317)) * `CrossChainEnabledPolygonChild`: replace the `require` statement with the custom error `NotCrossChainCall`. ([#3380](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3380)) - * `crosschain`: refactor the custom error parameters, which now start with the address of the emitter. ([#TBD]()) + * `crosschain`: refactor the custom error parameters, which now start with the address of the emitter. ([#3381](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3381)) ## 4.6.0 (2022-04-26)