Skip to content

Commit

Permalink
address issues raised in PR
Browse files Browse the repository at this point in the history
  • Loading branch information
nonergodic committed Nov 19, 2024
1 parent 69ec4b2 commit df355ae
Show file tree
Hide file tree
Showing 21 changed files with 309 additions and 210 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ For example, if you are using a solc version newer than `0.8.19` and are plannin

It is strongly recommended that you run the forge test suite of this SDK with your own compiler version to catch potential errors that stem from differences in compiler versions early. Yes, strictly speaking the Solidity version pragma should prevent these issues, but better to be safe than sorry, especially given that some components make extensive use of inline assembly.

**IERC20 Remapping**
**IERC20 and SafeERC20 Remapping**

This SDK comes with its own IERC20 interface. Given that projects tend to combine different SDKs, there's often this annoying issue of clashes of IERC20 interfaces, even though the are effectively the same. We handle this issue by importing `IERC20/IERC20.sol` which allows remapping the `IERC20/` prefix to whatever directory contains `IERC20.sol` in your project, thus providing an override mechanism that should allow dealing with this problem seamlessly until forge allows remapping of individual files.
This SDK comes with its own IERC20 interface and SafeERC20 implementation. Given that projects tend to combine different SDKs, there's often this annoying issue of clashes of IERC20 interfaces, even though the are effectively the same. We handle this issue by importing `IERC20/IERC20.sol` which allows remapping the `IERC20/` prefix to whatever directory contains `IERC20.sol` in your project, thus providing an override mechanism that should allow dealing with this problem seamlessly until forge allows remapping of individual files. The same approach is used for SafeERC20.

## Components

Expand Down
2 changes: 1 addition & 1 deletion foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ remappings = [
"forge-std/=lib/forge-std/src/",
"wormhole-sdk/=src/",
"IERC20/=src/interfaces/token/",
"@openzeppelin/=lib/openzeppelin-contracts/contracts/"
"SafeERC20/=src/libraries/",
]

verbosity = 3
26 changes: 0 additions & 26 deletions src/TransferUtils.sol

This file was deleted.

27 changes: 3 additions & 24 deletions src/Utils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,6 @@
// SPDX-License-Identifier: Apache 2
pragma solidity ^0.8.19;

error NotAnEvmAddress(bytes32);

function toUniversalAddress(address addr) pure returns (bytes32 universalAddr) {
universalAddr = bytes32(uint256(uint160(addr)));
}

function fromUniversalAddress(bytes32 universalAddr) pure returns (address addr) {
if (bytes12(universalAddr) != 0)
revert NotAnEvmAddress(universalAddr);

assembly ("memory-safe") {
addr := universalAddr
}
}

/**
* Reverts with a given buffer data.
* Meant to be used to easily bubble up errors from low level calls when they fail.
*/
function reRevert(bytes memory err) pure {
assembly ("memory-safe") {
revert(add(err, 32), mload(err))
}
}
import {tokenOrNativeTransfer} from "wormhole-sdk/utils/Transfer.sol";
import {reRevert} from "wormhole-sdk/utils/Revert.sol";
import {toUniversalAddress, fromUniversalAddress} from "wormhole-sdk/utils/UniversalAddress.sol";
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,19 @@
pragma solidity ^0.8.4;

import {BytesParsing} from "wormhole-sdk/libraries/BytesParsing.sol";
import "./ids.sol";
import {
ACCESS_CONTROL_ID,
ACCESS_CONTROL_QUERIES_ID,
OWNER_ID,
PENDING_OWNER_ID,
IS_ADMIN_ID,
ADMINS_ID,
REVOKE_ADMIN_ID,
ADD_ADMIN_ID,
PROPOSE_OWNERSHIP_TRANSFER_ID,
ACQUIRE_OWNERSHIP_ID,
RELINQUISH_OWNERSHIP_ID
} from "wormhole-sdk/components/dispatcher/Ids.sol";

//rationale for different roles (owner, admin):
// * owner should be a mulit-sig / ultra cold wallet that is only activated in exceptional
Expand Down Expand Up @@ -42,21 +54,24 @@ enum Role {
Admin
}

function senderHasAuth() view returns (Role) {
Role role = senderRole();
if (Role.None == role)
function failAuthIf(bool condition) pure {
if (condition)
revert NotAuthorized();
}

function senderAtLeastAdmin() view returns (Role) {
Role role = senderRole();
failAuthIf(role == Role.None);

return role;
}

function senderRole() view returns (Role) {
AccessControlState storage state = accessControlState();
if (msg.sender == state.owner) //check highest privilege level first
return Role.Owner;
else if (state.isAdmin[msg.sender] != 0)
return Role.Admin;
else
return Role.None;

return state.isAdmin[msg.sender] != 0 ? Role.Admin : Role.None;
}

abstract contract AccessControl {
Expand All @@ -77,16 +92,14 @@ abstract contract AccessControl {

function transferOwnership(address newOwner) external {
AccessControlState storage state = accessControlState();
if (msg.sender != state.owner)
revert NotAuthorized();
failAuthIf(msg.sender != state.owner);

state.pendingOwner = newOwner;
}

function cancelOwnershipTransfer() external {
AccessControlState storage state = accessControlState();
if (state.owner != msg.sender)
revert NotAuthorized();
failAuthIf(msg.sender != state.owner);

state.pendingOwner = address(0);
}
Expand All @@ -97,27 +110,31 @@ abstract contract AccessControl {

// ---- internals ----

/**
* Dispatch an execute function. Execute functions almost always modify contract state.
*/
function dispatchExecAccessControl(bytes calldata data, uint256 offset, uint8 command) internal returns (bool, uint256) {
function dispatchExecAccessControl(
bytes calldata data,
uint offset,
uint8 command
) internal returns (bool, uint) {
if (command == ACCESS_CONTROL_ID)
offset = _batchAccessControlCommands(data, offset);
else if (command == ACQUIRE_OWNERSHIP_ID)
_acquireOwnership();
else return (false, offset);
else
return (false, offset);

return (true, offset);
}

/**
* Dispatch a query function. Query functions never modify contract state.
*/
function dispatchQueryAccessControl(bytes calldata data, uint256 offset, uint8 query) view internal returns (bool, bytes memory, uint256) {
function dispatchQueryAccessControl(
bytes calldata data,
uint offset,
uint8 query
) view internal returns (bool, bytes memory, uint) {
bytes memory result;
if (query == ACCESS_CONTROL_QUERIES_ID)
(result, offset) = _batchAccessControlQueries(data, offset);
else return (false, new bytes(0), offset);
else
return (false, new bytes(0), offset);

return (true, result, offset);
}
Expand All @@ -127,7 +144,7 @@ abstract contract AccessControl {
uint offset
) internal returns (uint) {
AccessControlState storage state = accessControlState();
bool isOwner = senderHasAuth() == Role.Owner;
bool isOwner = senderAtLeastAdmin() == Role.Owner;

uint remainingCommands;
(remainingCommands, offset) = commands.asUint8CdUnchecked(offset);
Expand Down Expand Up @@ -180,12 +197,12 @@ abstract contract AccessControl {
for (uint i = 0; i < remainingQueries; ++i) {
uint8 query;
(query, offset) = queries.asUint8CdUnchecked(offset);

if (query == IS_ADMIN_ID) {
address admin;
(admin, offset) = queries.asAddressCdUnchecked(offset);
ret = abi.encodePacked(ret, state.isAdmin[admin] != 0);
}
}
else if (query == ADMINS_ID) {
ret = abi.encodePacked(ret, uint8(state.admins.length));
for (uint j = 0; j < state.admins.length; ++j)
Expand All @@ -209,7 +226,7 @@ abstract contract AccessControl {

function _acquireOwnership() internal {
AccessControlState storage state = accessControlState();
if (state.pendingOwner != msg.sender)
if (msg.sender !=state.pendingOwner)
revert NotAuthorized();

state.pendingOwner = address(0);
Expand Down
File renamed without changes.
37 changes: 37 additions & 0 deletions src/components/dispatcher/SweepTokens.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// SPDX-License-Identifier: Apache 2

pragma solidity ^0.8.4;

import {BytesParsing} from "wormhole-sdk/libraries/BytesParsing.sol";
import {tokenOrNativeTransfer} from "wormhole-sdk/utils/Transfer.sol";
import {senderAtLeastAdmin} from "wormhole-sdk/components/dispatcher/AccessControl.sol";
import {SWEEP_TOKENS_ID} from "wormhole-sdk/components/dispatcher/Ids.sol";

abstract contract SweepTokens {
using BytesParsing for bytes;

function dispatchExecSweepTokens(
bytes calldata data,
uint offset,
uint8 command
) internal returns (bool, uint) {
return command == SWEEP_TOKENS_ID
? (true, _sweepTokens(data, offset))
: (false, offset);
}

function _sweepTokens(
bytes calldata commands,
uint offset
) internal returns (uint) {
senderAtLeastAdmin();

address token;
uint256 amount;
(token, offset) = commands.asAddressCdUnchecked(offset);
(amount, offset) = commands.asUint256CdUnchecked(offset);

tokenOrNativeTransfer(token, msg.sender, amount);
return offset;
}
}
57 changes: 57 additions & 0 deletions src/components/dispatcher/Upgrade.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// SPDX-License-Identifier: Apache 2

pragma solidity ^0.8.4;

import {BytesParsing} from "wormhole-sdk/libraries/BytesParsing.sol";
import {ProxyBase} from "wormhole-sdk/proxy/ProxyBase.sol";
import {Role, senderRole, failAuthIf} from "wormhole-sdk/components/dispatcher/AccessControl.sol";
import {UPGRADE_CONTRACT_ID, IMPLEMENTATION_ID} from "wormhole-sdk/components/dispatcher/Ids.sol";

error InvalidGovernanceCommand(uint8 command);
error InvalidGovernanceQuery(uint8 query);

abstract contract Upgrade is ProxyBase {
using BytesParsing for bytes;

function dispatchExecUpgrade(
bytes calldata data,
uint offset,
uint8 command
) internal returns (bool, uint) {
return (command == UPGRADE_CONTRACT_ID)
? (true, _upgradeContract(data, offset))
: (false, offset);
}

function dispatchQueryUpgrade(
bytes calldata,
uint offset,
uint8 query
) view internal returns (bool, bytes memory, uint) {
return query == IMPLEMENTATION_ID
? (true, abi.encodePacked(_getImplementation()), offset)
: (false, new bytes(0), offset);
}

function upgrade(address implementation, bytes calldata data) external {
failAuthIf(senderRole() != Role.Owner);

_upgradeTo(implementation, data);
}

function _upgradeContract(
bytes calldata commands,
uint offset
) internal returns (uint) {
failAuthIf(senderRole() != Role.Owner);

address newImplementation;
(newImplementation, offset) = commands.asAddressCdUnchecked(offset);
//contract upgrades must be the last command in the batch
commands.checkLengthCd(offset);

_upgradeTo(newImplementation, new bytes(0));

return offset;
}
}
14 changes: 10 additions & 4 deletions src/constants/Common.sol
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
// SPDX-License-Identifier: Apache 2
pragma solidity ^0.8.4;

uint256 constant FREE_MEMORY_PTR = 0x40;
// ┌──────────────────────────────────────────────────────────────────────────────┐
// │ NOTE: We can't define e.g. WORD_SIZE_MINUS_ONE via WORD_SIZE - 1 because │
// │ of solc restrictions on what constants can be used in inline assembly. │
// └──────────────────────────────────────────────────────────────────────────────┘

uint256 constant WORD_SIZE = 32;
//we can't define _WORD_SIZE_MINUS_ONE via _WORD_SIZE - 1 because of solc restrictions
// what constants can be used in inline assembly
uint256 constant WORD_SIZE_MINUS_ONE = 31; //=0x1f=0b00011111

//see section "prefer `< MAX + 1` over `<= MAX` for const comparison" in docs/Optimization.md
uint256 constant WORD_SIZE_PLUS_ONE = 33;

uint256 constant SCRATCH_SPACE_PTR = 0x00;
uint256 constant SCRATCH_SPACE_SIZE = 64;

uint256 constant FREE_MEMORY_PTR = 0x40;
38 changes: 0 additions & 38 deletions src/dispatcherComponents/SweepTokens.sol

This file was deleted.

Loading

0 comments on commit df355ae

Please sign in to comment.