Skip to content

feat: optional allowlist for addresses that can interact with the inbox #685

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

Merged
PlasmaPower merged 5 commits into from
Jun 14, 2022
Merged

feat: optional allowlist for addresses that can interact with the inbox #685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a92a4dd
feat: optional allowlist for addresses that can interact with the inbox
fredlacs Jun 10, 2022
a51027c
fix: change to tx origin check
fredlacs Jun 10, 2022
b5d91c3
docs: add disclaimer about tx origin usage
fredlacs Jun 13, 2022
d96467f
refactor: optimise allowlist validation order and use solidity custom…
fredlacs Jun 14, 2022
3ac39bb
Merge branch 'master' into inbox-allowlist
PlasmaPower Jun 14, 2022
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
3 changes: 3 additions & 0 deletions contracts/src/bridge/IInbox.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ error InsufficientValue(uint256 expected, uint256 actual);
/// @dev submission cost provided isn't enough to create retryable ticket
error InsufficientSubmissionCost(uint256 expected, uint256 actual);

/// @dev address not allowed to interact with the given contract
error NotAllowedOrigin(address origin);

/// @dev used to convey retryable tx data in eth calls without requiring a tx trace
/// this follows a pattern similar to EIP-3668 where reverts surface call information
error RetryableData(
Expand Down
64 changes: 55 additions & 9 deletions contracts/src/bridge/Inbox.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,40 @@ import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox {
IBridge public override bridge;

/// ------------------------------------ allow list start ------------------------------------ ///

bool public allowListEnabled;
mapping(address => bool) public isAllowed;

event AllowListAddressSet(address indexed user, bool val);
event AllowListEnabledUpdated(bool isEnabled);

function setAllowList(address[] memory user, bool[] memory val) external onlyOwner {
require(user.length == val.length, "INVALID_INPUT");

for (uint256 i = 0; i < user.length; i++) {
isAllowed[user[i]] = val[i];
emit AllowListAddressSet(user[i], val[i]);
}
}

function setAllowListEnabled(bool _allowListEnabled) external onlyOwner {
require(_allowListEnabled != allowListEnabled, "ALREADY_SET");
allowListEnabled = _allowListEnabled;
emit AllowListEnabledUpdated(_allowListEnabled);
}

/// @dev this modifier checks the tx.origin instead of msg.sender for convenience (ie it allows
/// allowed users to interact with the token bridge without needing the token bridge to be allowList aware).
/// this modifier is not intended to use to be used for security (since this opens the allowList to
/// a smart contract phishing risk).
modifier onlyAllowed() {
if (allowListEnabled && !isAllowed[tx.origin]) revert NotAllowedOrigin(tx.origin);
_;
}

/// ------------------------------------ allow list end ------------------------------------ ///

modifier onlyOwner() {
// whoevever owns the Bridge, also owns the Inbox. this is usually the rollup contract
address bridgeOwner = OwnableUpgradeable(address(bridge)).owner();
Expand All @@ -52,6 +86,7 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox {
function initialize(IBridge _bridge) external initializer onlyDelegated {
if (address(bridge) != address(0)) revert AlreadyInit();
bridge = _bridge;
allowListEnabled = false;
__Pausable_init();
}

Expand All @@ -64,6 +99,7 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox {
sstore(i, 0)
}
}
allowListEnabled = false;
bridge = _bridge;
}

Expand All @@ -75,6 +111,7 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox {
function sendL2MessageFromOrigin(bytes calldata messageData)
external
whenNotPaused
onlyAllowed
returns (uint256)
{
// solhint-disable-next-line avoid-tx-origin
Expand All @@ -95,6 +132,7 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox {
external
override
whenNotPaused
onlyAllowed
returns (uint256)
{
return _deliverMessage(L2_MSG, msg.sender, messageData);
Expand All @@ -106,7 +144,7 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox {
uint256 nonce,
address to,
bytes calldata data
) external payable virtual override whenNotPaused returns (uint256) {
) external payable virtual override whenNotPaused onlyAllowed returns (uint256) {
return
_deliverMessage(
L1MessageType_L2FundedByL1,
Expand All @@ -128,7 +166,7 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox {
uint256 maxFeePerGas,
address to,
bytes calldata data
) external payable virtual override whenNotPaused returns (uint256) {
) external payable virtual override whenNotPaused onlyAllowed returns (uint256) {
return
_deliverMessage(
L1MessageType_L2FundedByL1,
Expand All @@ -151,7 +189,7 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox {
address to,
uint256 value,
bytes calldata data
) external virtual override whenNotPaused returns (uint256) {
) external virtual override whenNotPaused onlyAllowed returns (uint256) {
return
_deliverMessage(
L2_MSG,
Expand All @@ -174,7 +212,7 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox {
address to,
uint256 value,
bytes calldata data
) external virtual override whenNotPaused returns (uint256) {
) external virtual override whenNotPaused onlyAllowed returns (uint256) {
return
_deliverMessage(
L2_MSG,
Expand Down Expand Up @@ -209,7 +247,7 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox {
/// @dev this does not trigger the fallback function when receiving in the L2 side.
/// Look into retryable tickets if you are interested in this functionality.
/// @dev this function should not be called inside contract constructors
function depositEth() public payable override whenNotPaused returns (uint256) {
function depositEth() public payable override whenNotPaused onlyAllowed returns (uint256) {
address sender = msg.sender;

// solhint-disable-next-line avoid-tx-origin
Expand All @@ -232,7 +270,15 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox {
}

/// @notice deprecated in favour of depositEth with no parameters
function depositEth(uint256) external payable virtual override whenNotPaused returns (uint256) {
function depositEth(uint256)
external
payable
virtual
override
whenNotPaused
onlyAllowed
returns (uint256)
{
return depositEth();
}

Expand All @@ -259,7 +305,7 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox {
uint256 gasLimit,
uint256 maxFeePerGas,
bytes calldata data
) external payable virtual whenNotPaused returns (uint256) {
) external payable virtual whenNotPaused onlyAllowed returns (uint256) {
return
unsafeCreateRetryableTicket(
to,
Expand Down Expand Up @@ -296,7 +342,7 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox {
uint256 gasLimit,
uint256 maxFeePerGas,
bytes calldata data
) external payable virtual override whenNotPaused returns (uint256) {
) external payable virtual override whenNotPaused onlyAllowed returns (uint256) {
// ensure the user's deposit alone will make submission succeed
if (msg.value < maxSubmissionCost + l2CallValue)
revert InsufficientValue(maxSubmissionCost + l2CallValue, msg.value);
Expand Down Expand Up @@ -351,7 +397,7 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox {
uint256 gasLimit,
uint256 maxFeePerGas,
bytes calldata data
) public payable virtual override whenNotPaused returns (uint256) {
) public payable virtual override whenNotPaused onlyAllowed returns (uint256) {
// gas price and limit of 1 should never be a valid input, so instead they are used as
// magic values to trigger a revert in eth calls that surface data without requiring a tx trace
if (gasLimit == 1 || maxFeePerGas == 1)
Expand Down