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

Nonce workarounds #39

Merged
merged 14 commits into from
Apr 5, 2024
6 changes: 6 additions & 0 deletions per_multicall/src/Errors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,9 @@ error InvalidMagicValue();

// Signature: 0x0601f697
error InvalidFeeSplit();

// Signature: 0xb40d37c3
error DuplicateRelayerSubwallet();

// Signature: 0xac4d92b3
error RelayerSubwalletNotFound();
1 change: 1 addition & 0 deletions per_multicall/src/ExpressRelay.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ contract ExpressRelay is ExpressRelayHelpers, ExpressRelayState {
) {
state.admin = admin;
state.relayer = relayer;
state.relayerSubwallets = new address[](0);

validateFeeSplit(feeSplitProtocolDefault);
state.feeSplitProtocolDefault = feeSplitProtocolDefault;
Expand Down
66 changes: 65 additions & 1 deletion per_multicall/src/ExpressRelayState.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ contract ExpressRelayStorage {
struct State {
// address of admin of express relay, handles setting fees and relayer role
address admin;
// address of relayer EOA uniquely permissioned to call ExpressRelay.multicall
// address of primary relayer EOA, where relayer will ultimately receive fees
address relayer;
// store of relayer subwallets permissioned to call ExpressRelay.multicall
address[] relayerSubwallets;
// stores custom fee splits for protocol fee receivers
mapping(address => uint256) feeConfig;
// stores the flags for whether permission keys are currently allowed
Expand Down Expand Up @@ -40,6 +42,23 @@ contract ExpressRelayState is IExpressRelay {
}

modifier onlyRelayer() {
if (msg.sender != state.relayer) {
bool isSubwallet = false;
for (uint i = 0; i < state.relayerSubwallets.length; i++) {
if (state.relayerSubwallets[i] == msg.sender) {
isSubwallet = true;
break;
}
}

if (!isSubwallet) {
revert Unauthorized();
}
}
_;
}

modifier onlyRelayerPrimary() {
if (msg.sender != state.relayer) {
revert Unauthorized();
}
Expand All @@ -66,6 +85,7 @@ contract ExpressRelayState is IExpressRelay {
*/
function setRelayer(address relayer) public onlyAdmin {
state.relayer = relayer;
state.relayerSubwallets = new address[](0);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write some tests for this line of logic

}

/**
Expand All @@ -75,6 +95,50 @@ contract ExpressRelayState is IExpressRelay {
return state.relayer;
}

/**
* @notice addRelayerSubwallet function - adds a relayer subwallet
*
* @param subwallet: address of the relayer subwallet to be added
*/
function addRelayerSubwallet(address subwallet) public onlyRelayerPrimary {
for (uint i = 0; i < state.relayerSubwallets.length; i++) {
if (state.relayerSubwallets[i] == subwallet) {
revert DuplicateRelayerSubwallet();
}
}
state.relayerSubwallets.push(subwallet);
}

/**
* @notice removeRelayerSubwallet function - removes a relayer subwallet
*
* @param subwallet: address of the relayer subwallet to be removed
*/
function removeRelayerSubwallet(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add docs for this function too

address subwallet
) public onlyRelayerPrimary {
for (uint i = 0; i < state.relayerSubwallets.length; i++) {
if (state.relayerSubwallets[i] == subwallet) {
state.relayerSubwallets[i] = state.relayerSubwallets[
state.relayerSubwallets.length - 1
];
state.relayerSubwallets.pop();
break;
}

if (i == state.relayerSubwallets.length - 1) {
revert RelayerSubwalletNotFound();
}
}
}

/**
* @notice getRelayerSubwallets function - returns the relayer subwallets
*/
function getRelayerSubwallets() public view returns (address[] memory) {
return state.relayerSubwallets;
}

/**
* @notice setFeeProtocolDefault function - sets the default fee split for the protocol
*
Expand Down
103 changes: 102 additions & 1 deletion per_multicall/test/ExpressRelayUnit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,96 @@ contract ExpressRelayUnitTest is Test, ExpressRelayTestSetup {
expressRelay.setRelayer(newRelayer);
}

function testAddRelayerSubwalletByRelayerPrimary() public {
address subwallet = makeAddr("subwallet");
vm.prank(relayer);
expressRelay.addRelayerSubwallet(subwallet);
address[] memory relayerSubwallets = expressRelay
.getRelayerSubwallets();

assertAddressInArray(subwallet, relayerSubwallets, true);
}

function testAddRelayerSubwalletByNonRelayerPrimaryFail() public {
address subwallet1 = makeAddr("subwallet1");
vm.prank(relayer);
expressRelay.addRelayerSubwallet(subwallet1);

address subwallet2 = makeAddr("subwallet2");
vm.expectRevert(Unauthorized.selector);
vm.prank(subwallet1);
expressRelay.addRelayerSubwallet(subwallet2);
}

function testAddDuplicateRelayerSubwalletByRelayerPrimaryFail() public {
address subwallet = makeAddr("subwallet");
vm.prank(relayer);
expressRelay.addRelayerSubwallet(subwallet);
vm.expectRevert(DuplicateRelayerSubwallet.selector);
vm.prank(relayer);
expressRelay.addRelayerSubwallet(subwallet);
}

function testRemoveRelayerSubwalletByRelayerPrimary() public {
address subwallet1 = makeAddr("subwallet1");
address subwallet2 = makeAddr("subwallet2");
vm.prank(relayer);
expressRelay.addRelayerSubwallet(subwallet1);
vm.prank(relayer);
expressRelay.addRelayerSubwallet(subwallet2);
address[] memory relayerSubwalletsPre = expressRelay
.getRelayerSubwallets();

vm.prank(relayer);
expressRelay.removeRelayerSubwallet(subwallet1);
address[] memory relayerSubwalletsPost = expressRelay
.getRelayerSubwallets();

assertEq(relayerSubwalletsPre.length, relayerSubwalletsPost.length + 1);
assertAddressInArray(subwallet1, relayerSubwalletsPost, false);
assertAddressInArray(subwallet2, relayerSubwalletsPost, true);
}

function testRemoveRelayerSubwalletByNonRelayerPrimaryFail() public {
address subwallet1 = makeAddr("subwallet1");
address subwallet2 = makeAddr("subwallet2");
vm.prank(relayer);
expressRelay.addRelayerSubwallet(subwallet1);
vm.prank(relayer);
expressRelay.addRelayerSubwallet(subwallet2);

vm.expectRevert(Unauthorized.selector);
vm.prank(subwallet1);
expressRelay.removeRelayerSubwallet(subwallet2);
}

function testRemoveNonExistentRelayerSubwalletByRelayerFail() public {
address subwallet = makeAddr("subwallet");
vm.prank(relayer);
expressRelay.addRelayerSubwallet(subwallet);

address nonExistentSubwallet = makeAddr("nonExistentSubwallet");
vm.expectRevert(RelayerSubwalletNotFound.selector);
vm.prank(relayer);
expressRelay.removeRelayerSubwallet(nonExistentSubwallet);
}

function testChangeRelayerAfterAddingRelayerSubwallet() public {
address subwallet = makeAddr("subwallet");
vm.prank(relayer);
expressRelay.addRelayerSubwallet(subwallet);
address[] memory expectedSubwallets = new address[](1);
expectedSubwallets[0] = subwallet;
assertEq(expressRelay.getRelayerSubwallets(), expectedSubwallets);

address newRelayer = makeAddr("newRelayer");
vm.prank(admin);
expressRelay.setRelayer(newRelayer);

assertEq(expressRelay.getRelayer(), newRelayer);
assertEq(expressRelay.getRelayerSubwallets(), new address[](0));
}

function testSetFeeProtocolDefaultByAdmin() public {
uint256 feeSplitProtocolDefaultPre = expressRelay
.getFeeProtocolDefault();
Expand Down Expand Up @@ -130,7 +220,6 @@ contract ExpressRelayUnitTest is Test, ExpressRelayTestSetup {
uint256 feeMax = 10 ** 18;
vm.prank(admin);
expressRelay.setFeeRelayer(feeMax);
uint256 feeRelayerPost = expressRelay.getFeeRelayer();

// test setting fee to a value higher than the highest valid value, should fail
uint256 fee = 10 ** 18 + 1;
Expand All @@ -153,6 +242,18 @@ contract ExpressRelayUnitTest is Test, ExpressRelayTestSetup {
expressRelay.multicall(permission, multicallData);
}

function testMulticallByRelayerSubwalletEmpty() public {
address subwallet = makeAddr("subwallet");
vm.prank(relayer);
expressRelay.addRelayerSubwallet(subwallet);

bytes memory permission = abi.encode("random permission");
MulticallData[] memory multicallData;

vm.prank(subwallet);
expressRelay.multicall(permission, multicallData);
}

function testMulticallByNonRelayerFail() public {
bytes memory permission = abi.encode("random permission");
MulticallData[] memory multicallData;
Expand Down
15 changes: 15 additions & 0 deletions per_multicall/test/helpers/TestParsingHelpers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,21 @@ contract TestParsingHelpers is Test {
return BidInfo(bid, 1_000_000_000_000, vm.addr(executorSk), executorSk);
}

function assertAddressInArray(
address addr,
address[] memory arr,
bool exists
) internal pure {
bool found = false;
for (uint256 i = 0; i < arr.length; i++) {
if (arr[i] == addr) {
found = true;
break;
}
}
assert(found == exists);
}

function assertEqBalances(
AccountBalance memory a,
AccountBalance memory b
Expand Down
Loading