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.

Sign up for GitHub

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

feat(protocol): remove banning address #16604

Merged
merged 3 commits into from
Apr 3, 2024
Merged

feat(protocol): remove banning address #16604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a6470a1
remove banning address
dantaik Apr 2, 2024
788e95d
Merge branch 'main' into remove_address_banning
dantaik Apr 3, 2024
0d4135b
Merge branch 'main' into remove_address_banning
adaki2004 Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 2 additions & 19 deletions packages/protocol/contracts/bridge/Bridge.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ contract Bridge is EssentialContract, IBridge {

/// @notice Mapping to store banned addresses.
/// @dev Slot 5.
mapping(address addr => bool banned) public addressBanned;
uint256 private __reserved1;

/// @notice Mapping to store the proof receipt of a message from its hash.
/// @dev Slot 6.
Expand Down Expand Up @@ -110,23 +110,6 @@ contract Bridge is EssentialContract, IBridge {
}
}

/// @notice Ban or unban an address. A banned addresses will not be invoked upon
/// with message calls.
/// @dev Do not make this function `nonReentrant`, this breaks {DelegateOwner} support.
/// @param _addr The address to ban or unban.
/// @param _ban True if ban, false if unban.
function banAddress(
address _addr,
bool _ban
)
external
onlyFromOwnerOrNamed("bridge_watchdog")
{
if (addressBanned[_addr] == _ban) revert B_INVALID_STATUS();
addressBanned[_addr] = _ban;
emit AddressBanned(_addr, _ban);
}

/// @inheritdoc IBridge
function sendMessage(Message calldata _message)
external
Expand Down Expand Up @@ -292,7 +275,7 @@ contract Bridge is EssentialContract, IBridge {
// Process message differently based on the target address
if (
_message.to == address(0) || _message.to == address(this)
|| _message.to == signalService || addressBanned[_message.to]
|| _message.to == signalService
) {
// Handle special addresses that don't require actual invocation but
// mark message as DONE
Expand Down
76 changes: 0 additions & 76 deletions packages/protocol/test/bridge/Bridge.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,32 +369,6 @@ contract BridgeTest is TaikoTest {
assertEq(Carol.balance, 500);
}

function test_Bridge_banAddress_via_delegate_owner() public {
bytes memory banAddressCall = abi.encodeCall(Bridge.banAddress, (Alice, true));

IBridge.Message memory message = getDelegateOwnerMessage(
address(mockDAO),
abi.encodeCall(
DelegateOwner.onMessageInvocation,
abi.encode(0, address(destChainBridge), banAddressCall)
)
);

// Mocking proof - but obviously it needs to be created in prod
// corresponding to the message
bytes memory proof = hex"00";

bytes32 msgHash = destChainBridge.hashMessage(message);

vm.chainId(destChainId);

vm.prank(Bob, Bob);
destChainBridge.processMessage(message, proof);

IBridge.Status status = destChainBridge.messageStatus(msgHash);
assertEq(status == IBridge.Status.DONE, true);
}

function test_Bridge_pause_bridge_via_delegate_owner() public {
bytes memory pauseCall = abi.encodeCall(EssentialContract.pause, ());

Expand Down Expand Up @@ -483,33 +457,6 @@ contract BridgeTest is TaikoTest {
assertEq(status == IBridge.Status.DONE, true);
}

function test_Bridge_non_dao_cannot_call_via_delegate_owner() public {
bytes memory banAddressCall = abi.encodeCall(Bridge.banAddress, (Alice, true));

IBridge.Message memory message = getDelegateOwnerMessage(
Alice,
abi.encodeCall(
DelegateOwner.onMessageInvocation,
abi.encode(0, address(destChainBridge), banAddressCall)
)
);

// Mocking proof - but obviously it needs to be created in prod
// corresponding to the message
bytes memory proof = hex"00";

bytes32 msgHash = destChainBridge.hashMessage(message);

vm.chainId(destChainId);

vm.prank(Bob, Bob);
destChainBridge.processMessage(message, proof);

//Status retriable hence the low level call failed as from is not the DAO!
IBridge.Status status = destChainBridge.messageStatus(msgHash);
assertEq(status == IBridge.Status.RETRIABLE, true);
}

function test_Bridge_send_message_ether_reverts_if_value_doesnt_match_expected() public {
// uint256 amount = 1 wei;
IBridge.Message memory message = newMessage({
Expand Down Expand Up @@ -808,29 +755,6 @@ contract BridgeTest is TaikoTest {
assertEq(status == IBridge.Status.DONE, true);
}

function test_Bridge_ban_address() public {
vm.startPrank(Alice);
(IBridge.Message memory message, bytes memory proof) =
setUpPredefinedSuccessfulProcessMessageCall();

bytes32 msgHash = destChainBridge.hashMessage(message);
bytes32[] memory messageHashes = new bytes32[](1);
messageHashes[0] = msgHash;

vm.stopPrank();
// Ban address
vm.prank(destChainBridge.owner());
destChainBridge.banAddress(message.to, true);

vm.startPrank(Alice);
// processMessage() still marks it DONE but dont call the invokeMessageCall on them
destChainBridge.processMessage(message, proof);

IBridge.Status status = destChainBridge.messageStatus(msgHash);

assertEq(status == IBridge.Status.DONE, true);
}

function test_Bridge_prove_message_received() public {
vm.startPrank(Alice);
(IBridge.Message memory message, bytes memory proof) =
Expand Down
Loading