From d80bd32169930f2e6a969ed2e47173e4b6d6d2ed Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Thu, 15 Feb 2024 22:26:24 +0800 Subject: [PATCH] fix a bug found by DavidC --- .../contracts/signal/SignalService.sol | 17 +++++++++++++++-- packages/protocol/test/L1/TaikoL1TestBase.sol | 1 + packages/protocol/test/L2/TaikoL2.t.sol | 18 +++++++++++------- .../protocol/test/L2/TaikoL2NoFeeCheck.t.sol | 18 +++++++++++------- .../protocol/test/signal/SignalService.t.sol | 11 ++++++++++- 5 files changed, 48 insertions(+), 17 deletions(-) diff --git a/packages/protocol/contracts/signal/SignalService.sol b/packages/protocol/contracts/signal/SignalService.sol index a6685dd7f9..3c5f972f9a 100644 --- a/packages/protocol/contracts/signal/SignalService.sol +++ b/packages/protocol/contracts/signal/SignalService.sol @@ -39,12 +39,15 @@ contract SignalService is EssentialContract, ISignalService { bytes[] storageProof; } - uint256[50] private __gap; + mapping(address => bool) public isChainDataRelayerAuthorized; + uint256[49] private __gap; event SnippetRelayed( uint64 indexed chainid, bytes32 indexed kind, bytes32 data, bytes32 signal ); + event RelayerAuthorized(address indexed addr, bool authrized); + error SS_EMPTY_PROOF(); error SS_INVALID_APP(); error SS_INVALID_LAST_HOP_CHAINID(); @@ -53,12 +56,22 @@ contract SignalService is EssentialContract, ISignalService { error SS_INVALID_SIGNAL(); error SS_LOCAL_CHAIN_DATA_NOT_FOUND(); error SS_UNSUPPORTED(); + error SS_UNAUTHORIZED(); /// @dev Initializer to be called after being deployed behind a proxy. function init(address _addressManager) external initializer { __Essential_init(_addressManager); } + /// @dev Authorize or deautohrize an address for calling relayChainData + /// @dev Note that addr is supposed to be TaikoL1 and TaikoL1 contracts deployed locally. + function authorizeChainDataRelayer(address addr, bool toAuthorize) external onlyOwner { + if (isChainDataRelayerAuthorized[addr] == toAuthorize) revert SS_INVALID_PARAMS(); + isChainDataRelayerAuthorized[addr] = toAuthorize; + + emit RelayerAuthorized(addr, toAuthorize); + } + /// @inheritdoc ISignalService function relayChainData( uint64 chainId, @@ -66,9 +79,9 @@ contract SignalService is EssentialContract, ISignalService { bytes32 data ) external - onlyFromNamed("taiko") returns (bytes32 slot) { + if (!isChainDataRelayerAuthorized[msg.sender]) revert SS_UNAUTHORIZED(); return _relayChainData(chainId, kind, data); } diff --git a/packages/protocol/test/L1/TaikoL1TestBase.sol b/packages/protocol/test/L1/TaikoL1TestBase.sol index eb8ade7288..1131af447d 100644 --- a/packages/protocol/test/L1/TaikoL1TestBase.sol +++ b/packages/protocol/test/L1/TaikoL1TestBase.sol @@ -54,6 +54,7 @@ abstract contract TaikoL1TestBase is TaikoTest { data: abi.encodeCall(SignalService.init, address(addressManager)) }) ); + ss.authorizeChainDataRelayer(address(L1), true); pv = PseZkVerifier( deployProxy({ diff --git a/packages/protocol/test/L2/TaikoL2.t.sol b/packages/protocol/test/L2/TaikoL2.t.sol index 8f8dd5a9d2..e84616d001 100644 --- a/packages/protocol/test/L2/TaikoL2.t.sol +++ b/packages/protocol/test/L2/TaikoL2.t.sol @@ -28,13 +28,15 @@ contract TestTaikoL2 is TaikoTest { data: abi.encodeCall(AddressManager.init, ()) }); - deployProxy({ - name: "signal_service", - impl: address(new SignalService()), - data: abi.encodeCall(SignalService.init, (addressManager)), - registerTo: addressManager, - owner: address(0) - }); + SignalService ss = SignalService( + deployProxy({ + name: "signal_service", + impl: address(new SignalService()), + data: abi.encodeCall(SignalService.init, (addressManager)), + registerTo: addressManager, + owner: address(0) + }) + ); uint64 gasExcess = 0; uint8 quotient = 8; @@ -55,6 +57,8 @@ contract TestTaikoL2 is TaikoTest { L2.setConfigAndExcess(TaikoL2.Config(gasTarget, quotient), gasExcess); + ss.authorizeChainDataRelayer(address(L2), true); + gasExcess = 195_420_300_100; vm.roll(block.number + 1); diff --git a/packages/protocol/test/L2/TaikoL2NoFeeCheck.t.sol b/packages/protocol/test/L2/TaikoL2NoFeeCheck.t.sol index 062b34afe7..e84055bdfc 100644 --- a/packages/protocol/test/L2/TaikoL2NoFeeCheck.t.sol +++ b/packages/protocol/test/L2/TaikoL2NoFeeCheck.t.sol @@ -27,13 +27,15 @@ contract TestTaikoL2NoFeeCheck is TaikoTest { data: abi.encodeCall(AddressManager.init, ()) }); - deployProxy({ - name: "signal_service", - impl: address(new SignalService()), - data: abi.encodeCall(SignalService.init, (addressManager)), - registerTo: addressManager, - owner: address(0) - }); + SignalService ss = SignalService( + deployProxy({ + name: "signal_service", + impl: address(new SignalService()), + data: abi.encodeCall(SignalService.init, (addressManager)), + registerTo: addressManager, + owner: address(0) + }) + ); uint64 gasExcess = 0; uint8 quotient = 8; @@ -55,6 +57,8 @@ contract TestTaikoL2NoFeeCheck is TaikoTest { L2.setConfigAndExcess(TaikoL2.Config(gasTarget, quotient), gasExcess); + ss.authorizeChainDataRelayer(address(L2), true); + vm.roll(block.number + 1); vm.warp(block.timestamp + 30); } diff --git a/packages/protocol/test/signal/SignalService.t.sol b/packages/protocol/test/signal/SignalService.t.sol index 3a3983fb2e..1c8c0f1843 100644 --- a/packages/protocol/test/signal/SignalService.t.sol +++ b/packages/protocol/test/signal/SignalService.t.sol @@ -51,7 +51,9 @@ contract TestSignalService is TaikoTest { ); taiko = randAddress(); - addressManager.setAddress(uint64(block.chainid), "taiko", taiko); + + signalService.authorizeChainDataRelayer(taiko, true); + vm.stopPrank(); } @@ -298,6 +300,13 @@ contract TestSignalService is TaikoTest { signal: randBytes32(), proof: abi.encode(proofs) }); + + vm.prank(Alice); + signalService.authorizeChainDataRelayer(taiko, false); + + vm.expectRevert(SignalService.SS_UNAUTHORIZED.selector); + vm.prank(taiko); + signalService.relayChainData(srcChainId, LibSignals.SIGNAL_ROOT, proofs[0].rootHash); } function test_SignalService_proveSignalReceived_one_hop_state_root() public {