Wormhole - Ethereum

The `_transferTokens()` function normalizes the transferred token amount by setting the last 8 digit as zero via `deNormalizeAmount()` and `normalizeAmount()`. ```solidity=136 // don't deposit dust that can not be bridged due to the decimal shift amount = deNormalizeAmount(normalizeAmount(amount, decimals), decimals); ``` When transferring a standard ERC20 token, the normalization in L137 will ensure the transferred amount (calculated in L152) aligns with the `transferResult` specified in L171. ```solidity=152 amount = balanceAfter - balanceBefore; ``` ```solidity=168 transferResult = BridgeStructs.TransferResult({ tokenChain : tokenChain, tokenAddress : tokenAddress, normalizedAmount : normalizedAmount, normalizedArbiterFee : normalizedArbiterFee, wormholeFee : msg.value }); ``` However, when transferring tokens that charge fees during transfer (i.e., deflationary tokens), the normalizing process in L137 cannot ensure the amount calculated in L152 is normalized because of the fee charged. Therefore, normalizing the result in L160 could lead to a potential precision loss. ```solidity=160 uint256 normalizedAmount = normalizeAmount(amount, decimals); ``` In this case, a small number of deflationary tokens will be left in the contract. Since the contract does not provide a method to withdraw these locked tokens, these tokens will be locked in the contract forever.

One or more declarations do not conform to the [Solidity style guide](https://solidity.readthedocs.io/en/v0.7.3/style-guide.html#naming-conventions) with regards to its naming convention. Particularly: - `UPPER_CASE`: Should be applied to `constant` variables --- ```solidity=18 bytes32 constant module = 0x00000000000000000000000000000000000000000000000000000000436f7265; ``` - Constant variable `module` is not in `UPPER_CASE`. --- ```solidity=24 bytes32 constant module = 0x00000000000000000000000000000000000000000000004e4654427269646765; ``` - Constant variable `module` is not in `UPPER_CASE`. --- ```solidity=25 bytes32 constant module = 0x000000000000000000000000000000000000000000546f6b656e427269646765; ``` - Constant variable `module` is not in `UPPER_CASE`.

Multiple Solidity versions are used in the codebase. Versions used: `^0.8.0`, `^0.8.2`, `>=0.8.0<0.9.0`, `>=0.4.22<0.9.0`, `^0.8.1` Other directives used: `ABIEncoderV2` `^0.8.0` is used in ethereum/contracts/bridge/BridgeGovernance.sol file. ```solidity=4 pragma solidity ^0.8.0; ``` `^0.8.2` is used in ethereum/node_modules/@openzeppelin/contracts/proxy/ERC1967/ERC1967Upgrade.sol file. ```solidity=4 pragma solidity ^0.8.2; ``` `>=0.8.0<0.9.0` is used in ethereum/contracts/libraries/external/BytesLib.sol file. ```solidity=9 pragma solidity >=0.8.0 <0.9.0; ``` `>=0.4.22<0.9.0` is used in ethereum/contracts/Migrations.sol file. ```solidity=2 pragma solidity >=0.4.22 <0.9.0; ``` `^0.8.1` is used in ethereum/node_modules/@openzeppelin/contracts/utils/Address.sol file. ```solidity=4 pragma solidity ^0.8.1; ``` `ABIEncoderV2` is used in ethereum/contracts/Messages.sol file. ```solidity=5 pragma experimental ABIEncoderV2; ```

The functions which are never called internally within the contract should have external visibility for gas optimization. ```solidity=100 function completeTransfer(bytes memory encodedVm) public { ``` --- ```solidity=102 function setApprovalForAll(address operator, bool approved) public override { ``` --- ```solidity=109 function transferTokens(address token, uint256 amount, uint16 recipientChain, bytes32 recipient, uint256 arbiterFee, uint32 nonce) public payable nonReentrant returns (uint64 sequence) { ``` --- ```solidity=113 function transferFrom( ``` --- ```solidity=114 function transferTokensWithPayload(address token, uint256 amount, uint16 recipientChain, bytes32 recipient, uint256 arbiterFee, uint32 nonce, bytes memory payload) public payable nonReentrant returns (uint64 sequence) { ``` --- ```solidity=119 function mint(address account_, uint256 amount_) public onlyOwner { ``` --- ```solidity=12 function setup( ``` --- ```solidity=12 function setup( ``` --- ```solidity=12 function setup( ``` --- ```solidity=124 function safeTransferFrom( ``` --- ```solidity=131 function burn(address account_, uint256 amount_) public onlyOwner { ``` --- ```solidity=14 function implementation() public view returns (address) { ``` --- ```solidity=14 function implementation() public view returns (address) { ``` --- ```solidity=15 function publishMessage( ``` --- ```solidity=154 function updateDetails(string memory name_, string memory symbol_, uint64 sequence_) public onlyOwner { ``` --- ```solidity=16 function initialize( ``` --- ```solidity=16 function parseAndVerifyVM(bytes calldata encodedVM) public view returns (Structs.VM memory vm, bool valid, string memory reason) { ``` --- ```solidity=16 function setCompleted(uint completed) public restricted { ``` --- ```solidity=162 function mint(address to, uint256 tokenId, string memory uri) public onlyOwner { ``` --- ```solidity=17 function getGuardianSetExpiry() public view returns (uint32) { ``` --- ```solidity=177 function burn(uint256 tokenId) public onlyOwner { ``` --- ```solidity=18 function initialize() initializer public virtual { ``` --- ```solidity=18 function initialize() initializer public virtual { ``` --- ```solidity=20 function submitContractUpgrade(bytes memory _vm) public { ``` --- ```solidity=22 function initialize( ``` --- ```solidity=23 function transferNFT(address token, uint256 tokenID, uint16 recipientChain, bytes32 recipient, uint32 nonce) public payable returns (uint64 sequence) { ``` --- ```solidity=24 function attestToken(address tokenAddress, uint32 nonce) public payable returns (uint64 sequence){ ``` --- ```solidity=27 function registerChain(bytes memory encodedVM) public { ``` --- ```solidity=28 function registerChain(bytes memory encodedVM) public { ``` --- ```solidity=300 function completeTransferWithPayload(bytes memory encodedVm, address feeRecipient) public returns (bytes memory) { ``` --- ```solidity=304 function completeTransferAndUnwrapETHWithPayload(bytes memory encodedVm, address feeRecipient) public returns (bytes memory) { ``` --- ```solidity=308 function completeTransfer(bytes memory encodedVm) public { ``` --- ```solidity=312 function completeTransferAndUnwrapETH(bytes memory encodedVm) public { ``` --- ```solidity=36 function submitSetMessageFee(bytes memory _vm) public { ``` --- ```solidity=38 function name() public view returns (string memory) { ``` --- ```solidity=41 function upgrade(bytes memory encodedVM) public { ``` --- ```solidity=42 function symbol() public view returns (string memory) { ``` --- ```solidity=43 function upgrade(bytes memory encodedVM) public { ``` --- ```solidity=47 function balanceOf(address owner_) public view override returns (uint256) { ``` --- ```solidity=50 function decimals() public view returns (uint8) { ``` --- ```solidity=52 function submitNewGuardianSet(bytes memory _vm) public { ``` --- ```solidity=54 function totalSupply() public view returns (uint256) { ``` --- ```solidity=58 function chainId() public view returns (uint16) { ``` --- ```solidity=58 function name() public view override returns (string memory) { ``` --- ```solidity=62 function nativeContract() public view returns (bytes32) { ``` --- ```solidity=62 function symbol() public view override returns (string memory) { ``` --- ```solidity=64 function wrapAndTransferETH(uint16 recipientChain, bytes32 recipient, uint256 arbiterFee, uint32 nonce) public payable returns (uint64 sequence) { ``` --- ```solidity=66 function balanceOf(address account_) public view returns (uint256) { ``` --- ```solidity=66 function tokenURI(uint256 tokenId) public view override returns (string memory) { ``` --- ```solidity=69 function wrapAndTransferETHWithPayload(u

When expiring a guardian set, the guardian set is set to expire after a day. ```solidity=13 function expireGuardianSet(uint32 index) internal { _state.guardianSets[index].expirationTime = uint32(block.timestamp) + 86400; } ``` However, the `WormholeState` struct has a `guardianSetExpiry` field, which decides how long it takes for guardian sets to expire. Since the value in this field may be different from a day, guardian sets may expire earlier or later than expected.

When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. For example, if `toAsset` is a deflationary token and a user adds 100 deflationary tokens (with a 10% transaction fee) to the contract, 100 LP tokens are minted but only 90 tokens actually arrived in the contract. However, when the user wants to burn 100 LP tokens and withdraw 100 tokens, the program blocks due to a lack of `toAsset` tokens.

The `parseAndVerifyVM()` method from the core bridge of Wormhole decodes `encodedVm` and returns `vm` as the output. Then `_completeTransfer` will decode `vm.payload` and check whether `vm` is completed or not by its hash attribute. ```solidity (IWormhole.VM memory vm, bool valid, string memory reason) = wormhole().parseAndVerifyVM(encodedVm); require(valid, reason); require(verifyBridgeVM(vm), "invalid emitter"); BridgeStructs.Transfer memory transfer = parseTransfer(vm.payload); // payload 3 must be redeemed by the designated proxy contract address transferRecipient = address(uint160(uint256(transfer.to))); if (transfer.payloadID == 3) { require(msg.sender == transferRecipient, "invalid sender"); } require(!isTransferCompleted(vm.hash), "transfer already completed"); setTransferCompleted(vm.hash); ``` If the transfer is already completed, the invocation will revert in the require statement `require(!isTransferCompleted(vm.hash), "transfer already completed");`. However, considering the gas consumption, it is recommended to perform an early revert to save gas.

In the Wormhole guardian node, `Unmarshal(data []byte)` from `structs.go` will get the first 1000 bytes from the payload. ```go func Unmarshal(data []byte) (*VAA, error) { //... payload := make([]byte, 1000) n, err := reader.Read(payload) if err != nil || n == 0 { return nil, fmt.Errorf("failed to read payload [%d]: %w", n, err) } v.Payload = payload[:n] return v, nil } ``` In addition, there is also a size limit of 1232 bytes for transactions containing the `payload` on Solana, see [Solana Facts](https://solanacookbook.com/core-concepts/transactions.html#facts). However, there is no size limit for the input `payload` in `transferTokensWithPayload` or `wrapAndTransferETHWithPayload` and thus part of the `payload` info may be lost when doing a cross-chain transaction.

According to the following code, the check on `messageFee` in the function `publishMessage()` uses `==` to check whether the `msg.value` is equal to the `messageFee`. ```solidity function publishMessage( uint32 nonce, bytes memory payload, uint8 consistencyLevel ) public payable returns (uint64 sequence) { // check fee require(msg.value == messageFee(), "invalid fee"); sequence = useSequence(msg.sender); // emit log emit LogMessagePublished(msg.sender, sequence, nonce, payload, consistencyLevel); } ``` The function `publishMessage()` is called in the functions `Bridge.attestToken()`, `Bridge.logTransfer()`, `Bridge.logTransferWithPayload()` and `NFTBridge.logTransfer()`. In these calls, the `msg.value` is not checked or limited. As a result, if `msg.value` is not equal to `messageFee`, the call will be rolled back and all processes will be invalidated. We would like to confirm with the client if the current implementation aligns with the original project design.

The comment on `splCache` describes that `splCache` is a nested `mapping`, while the code implies otherwise.