From 75c6427d7c02a1ada5412a4e8b59c7f013038072 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Mon, 27 Jan 2025 04:23:50 -0500 Subject: [PATCH] CREATE uses RLP, not ABI-encoding --- .../ethereum/router/contracts/Router.sol | 93 +++++++++++++---- processor/ethereum/router/src/gas.rs | 10 +- processor/ethereum/router/src/tests/mod.rs | 99 +++++++------------ 3 files changed, 115 insertions(+), 87 deletions(-) diff --git a/processor/ethereum/router/contracts/Router.sol b/processor/ethereum/router/contracts/Router.sol index 3bd5c73f..79d01226 100644 --- a/processor/ethereum/router/contracts/Router.sol +++ b/processor/ethereum/router/contracts/Router.sol @@ -35,17 +35,6 @@ contract Router is IRouterWithoutCollisions { /// @dev The address in transient storage used for the reentrancy guard bytes32 constant REENTRANCY_GUARD_SLOT = bytes32(uint256(keccak256("ReentrancyGuard Router")) - 1); - /** - * @dev The next nonce used to determine the address of contracts deployed with CREATE. This is - * used to predict the addresses of deployed contracts ahead of time. - */ - /* - We don't expose a getter for this as it shouldn't be expected to have any specific value at a - given moment in time. If someone wants to know the address of their deployed contract, they can - have it emit an event and verify the emitting contract is the expected one. - */ - uint256 private _smartContractNonce; - /** * @dev The nonce to verify the next signature with, incremented upon an action to prevent * replays/out-of-order execution @@ -64,6 +53,17 @@ contract Router is IRouterWithoutCollisions { */ bytes32 private _seraiKey; + /** + * @dev The next nonce used to determine the address of contracts deployed with CREATE. This is + * used to predict the addresses of deployed contracts ahead of time. + */ + /* + We don't expose a getter for this as it shouldn't be expected to have any specific value at a + given moment in time. If someone wants to know the address of their deployed contract, they can + have it emit an event and verify the emitting contract is the expected one. + */ + uint64 private _smartContractNonce; + /// @dev The address escaped to address private _escapedTo; @@ -84,6 +84,7 @@ contract Router is IRouterWithoutCollisions { // Clear the re-entrancy guard to allow multiple transactions to non-re-entrant functions within // a transaction + // slither-disable-next-line assembly assembly { tstore(reentrancyGuardSlot, 0) } @@ -163,8 +164,8 @@ contract Router is IRouterWithoutCollisions { bytes32 signatureC; bytes32 signatureS; - // slither-disable-next-line assembly uint256 chainID = block.chainid; + // slither-disable-next-line assembly assembly { // Read the signature (placed after the function signature) signatureC := mload(add(message, 36)) @@ -402,6 +403,64 @@ contract Router is IRouterWithoutCollisions { } } + /// @notice The header for an address, when encoded with RLP for the purposes of CREATE + /// @dev 0x80 + 20, shifted left 30 bytes + uint256 constant ADDRESS_HEADER = (0x80 + 20) << (30 * 8); + + /// @notice Calculate the next address which will be deployed to by CREATE + /** + * @dev This manually implements the RLP encoding to save gas over the usage of CREATE2. While the + * the keccak256 call itself is surprisingly cheap, the memory cost (quadratic and already + * detrimental to other `OutInstruction`s within the same batch) is sufficiently concerning to + * justify this. + */ + function createAddress(uint256 nonce) private view returns (address) { + unchecked { + /* + The hashed RLP-encoding is: + - Header (1 byte) + - Address header (1 bytes) + - Address (20 bytes) + - Nonce (1 ..= 9 bytes) + Since the maximum length is less than 32 bytes, we calculate this on the stack. + */ + // Shift the address from bytes 12 .. 32 to 2 .. 22 + uint256 rlpEncoding = uint256(uint160(address(this))) << 80; + uint256 rlpEncodingLen; + if (nonce <= 0x7f) { + // 22 + 1 + rlpEncodingLen = 23; + // Shift from byte 31 to byte 22 + rlpEncoding |= (nonce << 72); + } else { + uint256 bitsNeeded = 8; + while (nonce >= (1 << bitsNeeded)) { + bitsNeeded += 8; + } + uint256 bytesNeeded = bitsNeeded / 8; + rlpEncodingLen = 22 + bytesNeeded; + // Shift from byte 31 to byte 22 + rlpEncoding |= 0x80 + (bytesNeeded << 72); + // Shift past the unnecessary bytes + rlpEncoding |= nonce << (72 - bitsNeeded); + } + rlpEncoding |= ADDRESS_HEADER; + // The header, which does not include itself in its length, shifted into the first byte + rlpEncoding |= (0xc0 + (rlpEncodingLen - 1)) << 248; + + // Store this to the scratch space + bytes memory rlp; + // slither-disable-next-line assembly + assembly { + mstore(0, rlpEncodingLen) + mstore(32, rlpEncoding) + rlp := 0 + } + + return address(uint160(uint256(keccak256(rlp)))); + } + } + /// @notice Execute some arbitrary code within a secure sandbox /** * @dev This performs sandboxing by deploying this code with `CREATE`. This is an external @@ -473,12 +532,12 @@ contract Router is IRouterWithoutCollisions { /* If it's an ERC20, we calculate the address of the will-be contract and transfer to it before deployment. This avoids needing to deploy the contract, then call transfer, then - call the contract again - */ - address nextAddress = address( - uint160(uint256(keccak256(abi.encodePacked(address(this), _smartContractNonce)))) - ); + call the contract again. + We use CREATE, not CREATE2, despite the difficulty in calculating the address + in-contract, for cost-savings reasons explained within `createAddress`'s documentation. + */ + address nextAddress = createAddress(_smartContractNonce); success = erc20TransferOut(nextAddress, coin, outs[i].amount); } diff --git a/processor/ethereum/router/src/gas.rs b/processor/ethereum/router/src/gas.rs index 769a2010..c3deb022 100644 --- a/processor/ethereum/router/src/gas.rs +++ b/processor/ethereum/router/src/gas.rs @@ -23,8 +23,8 @@ const CHAIN_ID: U256 = U256::from_be_slice(&[1]); pub(crate) type GasEstimator = Evm<'static, (), InMemoryDB>; impl Router { - const NONCE_STORAGE_SLOT: U256 = U256::from_be_slice(&[1]); - const SERAI_KEY_STORAGE_SLOT: U256 = U256::from_be_slice(&[3]); + const NONCE_STORAGE_SLOT: U256 = U256::from_be_slice(&[0]); + const SERAI_KEY_STORAGE_SLOT: U256 = U256::from_be_slice(&[2]); // Gas allocated for ERC20 calls #[cfg(test)] @@ -46,11 +46,11 @@ impl Router { the correct set of prices for the network they're operating on. */ /// The gas used by `confirmSeraiKey`. - pub const CONFIRM_NEXT_SERAI_KEY_GAS: u64 = 57_736; + pub const CONFIRM_NEXT_SERAI_KEY_GAS: u64 = 57_764; /// The gas used by `updateSeraiKey`. - pub const UPDATE_SERAI_KEY_GAS: u64 = 60_045; + pub const UPDATE_SERAI_KEY_GAS: u64 = 60_073; /// The gas used by `escapeHatch`. - pub const ESCAPE_HATCH_GAS: u64 = 61_094; + pub const ESCAPE_HATCH_GAS: u64 = 44_037; /// The key to use when performing gas estimations. /// diff --git a/processor/ethereum/router/src/tests/mod.rs b/processor/ethereum/router/src/tests/mod.rs index c0e43685..61572e6e 100644 --- a/processor/ethereum/router/src/tests/mod.rs +++ b/processor/ethereum/router/src/tests/mod.rs @@ -465,6 +465,8 @@ impl Test { self.provider.debug_trace_transaction(*tx.hash(), Default::default()).await.unwrap(); let refund = trace.try_into_default_frame().unwrap().struct_logs.last().unwrap().refund_counter; + // This isn't capped to 1/5th of the TX's gas usage yet that's fine as none of our tests are + // so refund intensive unused_gas += refund.unwrap_or(0) } @@ -881,7 +883,37 @@ async fn test_eth_code_out_instruction() { #[tokio::test] async fn test_erc20_code_out_instruction() { - todo!("TODO") + let mut test = Test::new().await; + test.confirm_next_serai_key().await; + + let erc20 = Erc20::deploy(&test).await; + let coin = Coin::Erc20(erc20.address()); + + let mut rand_address = [0xff; 20]; + OsRng.fill_bytes(&mut rand_address); + let amount_out = U256::from(2); + let out_instructions = OutInstructions::from( + [( + SeraiEthereumAddress::Contract(ContractDeployment::new(50_000, vec![]).unwrap()), + amount_out, + )] + .as_slice(), + ); + + let gas = test.router.execute_gas(coin, U256::from(1), &out_instructions); + let fee = U256::from(gas); + + // Mint to the Router the necessary amount of the ERC20 + erc20.mint(&test, test.router.address(), amount_out + fee).await; + + let (tx, gas_used) = test.execute(coin, fee, out_instructions, vec![true]).await; + + let unused_gas = test.gas_unused_by_calls(&tx).await; + assert_eq!(gas_used + unused_gas, gas); + + assert_eq!(erc20.balance_of(&test, test.router.address()).await, U256::from(0)); + assert_eq!(erc20.balance_of(&test, tx.recover_signer().unwrap()).await, U256::from(fee)); + assert_eq!(erc20.balance_of(&test, test.router.address().create(1)).await, amount_out); } #[tokio::test] @@ -1006,68 +1038,5 @@ async fn test_escape_hatch() { error Reentered(); error EscapeFailed(); function executeArbitraryCode(bytes memory code) external payable; - enum DestinationType { - Address, - Code - } - struct CodeDestination { - uint32 gasLimit; - bytes code; - } - struct OutInstruction { - DestinationType destinationType; - bytes destination; - uint256 amount; - } - function execute( - Signature calldata signature, - address coin, - uint256 fee, - OutInstruction[] calldata outs - ) external; -} - -async fn publish_outs( - provider: &RootProvider, - router: &Router, - key: (Scalar, PublicKey), - nonce: u64, - coin: Coin, - fee: U256, - outs: OutInstructions, -) -> TransactionReceipt { - let msg = Router::execute_message(nonce, coin, fee, outs.clone()); - - let nonce = Scalar::random(&mut OsRng); - let c = Signature::challenge(ProjectivePoint::GENERATOR * nonce, &key.1, &msg); - let s = nonce + (c * key.0); - - let sig = Signature::new(c, s).unwrap(); - - let mut tx = router.execute(coin, fee, outs, &sig); - tx.gas_price = 100_000_000_000; - let tx = ethereum_primitives::deterministically_sign(tx); - ethereum_test_primitives::publish_tx(provider, tx).await -} - -#[tokio::test] -async fn test_eth_address_out_instruction() { - let (_anvil, provider, router, key) = setup_test().await; - confirm_next_serai_key(&provider, &router, 1, key).await; - - let mut amount = U256::try_from(OsRng.next_u64()).unwrap(); - let mut fee = U256::try_from(OsRng.next_u64()).unwrap(); - if fee > amount { - core::mem::swap(&mut amount, &mut fee); - } - assert!(amount >= fee); - ethereum_test_primitives::fund_account(&provider, router.address(), amount).await; - - let instructions = OutInstructions::from([].as_slice()); - let receipt = publish_outs(&provider, &router, key, 2, Coin::Ether, fee, instructions).await; - assert!(receipt.status()); - assert_eq!(Router::EXECUTE_ETH_BASE_GAS, ((receipt.gas_used + 1000) / 1000) * 1000); - - assert_eq!(router.next_nonce(receipt.block_hash.unwrap().into()).await.unwrap(), 3); -} + function createAddress(uint256 nonce) private view returns (address); */