Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom errors start with the address of the emitter #3381

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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 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)

Expand Down
4 changes: 2 additions & 2 deletions contracts/crosschain/CrossChainEnabled.sol
Expand Up @@ -23,7 +23,7 @@ abstract contract CrossChainEnabled {
* cross-chain execution.
*/
modifier onlyCrossChain() {
if (!_isCrossChain()) revert NotCrossChainCall();
if (!_isCrossChain()) revert NotCrossChainCall(address(this));
_;
}

Expand All @@ -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);
_;
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/crosschain/amb/LibAMB.sol
Expand Up @@ -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();
}
}
2 changes: 1 addition & 1 deletion contracts/crosschain/arbitrum/LibArbitrumL1.sol
Expand Up @@ -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");
Expand Down
2 changes: 1 addition & 1 deletion contracts/crosschain/arbitrum/LibArbitrumL2.sol
Expand Up @@ -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()
Expand Down
17 changes: 15 additions & 2 deletions contracts/crosschain/errors.sol
Expand Up @@ -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);
2 changes: 1 addition & 1 deletion contracts/crosschain/optimism/LibOptimism.sol
Expand Up @@ -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();
}
Expand Down
Expand Up @@ -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");
Expand Down
4 changes: 2 additions & 2 deletions contracts/mocks/crosschain/bridges.sol
Expand Up @@ -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;

Expand Down
4 changes: 2 additions & 2 deletions test/crosschain/CrossChainEnabled.test.js
Expand Up @@ -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()}")`,
);
});

Expand Down