diff --git a/processor/ethereum/router/src/lib.rs b/processor/ethereum/router/src/lib.rs index 42495b13..c17526c3 100644 --- a/processor/ethereum/router/src/lib.rs +++ b/processor/ethereum/router/src/lib.rs @@ -68,6 +68,14 @@ use abi::{ #[cfg(test)] mod tests; +// As per Dencun, used for estimating gas for determining relayer fees +const NON_ZERO_BYTE_GAS_COST: u64 = 16; +const MEMORY_EXPANSION_COST: u64 = 3; // Does not model the quadratic cost +const COLD_COST: u64 = 2_600; +const WARM_COST: u64 = 100; +const POSITIVE_VALUE_COST: u64 = 9_000; +const EMPTY_ACCOUNT_COST: u64 = 25_000; + impl From<&Signature> for abi::Signature { fn from(signature: &Signature) -> Self { Self { @@ -134,35 +142,33 @@ pub struct InInstruction { pub data: Vec, } +impl From<&(SeraiAddress, U256)> for abi::OutInstruction { + fn from((address, amount): &(SeraiAddress, U256)) -> Self { + #[allow(non_snake_case)] + let (destinationType, destination) = match address { + SeraiAddress::Address(address) => { + // Per the documentation, `DestinationType::Address`'s value is an ABI-encoded address + (abi::DestinationType::Address, (Address::from(address)).abi_encode()) + } + SeraiAddress::Contract(contract) => ( + abi::DestinationType::Code, + (abi::CodeDestination { + gasLimit: contract.gas_limit(), + code: contract.code().to_vec().into(), + }) + .abi_encode(), + ), + }; + abi::OutInstruction { destinationType, destination: destination.into(), amount: *amount } + } +} + /// A list of `OutInstruction`s. #[derive(Clone)] pub struct OutInstructions(Vec); impl From<&[(SeraiAddress, U256)]> for OutInstructions { fn from(outs: &[(SeraiAddress, U256)]) -> Self { - Self( - outs - .iter() - .map(|(address, amount)| { - #[allow(non_snake_case)] - let (destinationType, destination) = match address { - SeraiAddress::Address(address) => { - // Per the documentation, `DestinationType::Address`'s value is an ABI-encoded - // address - (abi::DestinationType::Address, (Address::from(address)).abi_encode()) - } - SeraiAddress::Contract(contract) => ( - abi::DestinationType::Code, - (abi::CodeDestination { - gasLimit: contract.gas_limit(), - code: contract.code().to_vec().into(), - }) - .abi_encode(), - ), - }; - abi::OutInstruction { destinationType, destination: destination.into(), amount: *amount } - }) - .collect(), - ) + Self(outs.iter().map(Into::into).collect()) } } @@ -189,6 +195,8 @@ pub enum Executed { nonce: u64, /// The hash of the signed message for the Batch executed. message_hash: [u8; 32], + /// The results of the `OutInstruction`s executed. + results: Vec, }, /// The escape hatch was set. EscapeHatch { @@ -238,12 +246,15 @@ pub struct Router { address: Address, } impl Router { + // Gas allocated for ERC20 calls + const GAS_FOR_ERC20_CALL: u64 = 100_000; + /* The gas limits to use for transactions. - These are expected to be constant as a distributed group signs the transactions invoking these - calls. Having the gas be constant prevents needing to run a protocol to determine what gas to - use. + These are expected to be constant as a distributed group may sign the transactions invoking + these calls. Having the gas be constant prevents needing to run a protocol to determine what + gas to use. These gas limits may break if/when gas opcodes undergo repricing. In that case, this library is expected to be modified with these made parameters. The caller would then be expected to pass @@ -251,9 +262,18 @@ impl Router { */ const CONFIRM_NEXT_SERAI_KEY_GAS: u64 = 57_736; const UPDATE_SERAI_KEY_GAS: u64 = 60_045; - const EXECUTE_BASE_GAS: u64 = 48_000; + const EXECUTE_BASE_GAS: u64 = 51_131; const ESCAPE_HATCH_GAS: u64 = 61_238; + /* + The percentage to actually use as the gas limit, in case any opcodes are repriced or errors + occurred. + + Per prior commentary, this is just intended to be best-effort. If this is unnecessary, the gas + will be unspent. If this becomes necessary, it avoids needing an update. + */ + const GAS_REPRICING_BUFFER: u64 = 120; + fn code() -> Vec { const BYTECODE: &[u8] = { const BYTECODE_HEX: &[u8] = @@ -325,7 +345,7 @@ impl Router { TxLegacy { to: TxKind::Call(self.address), input: abi::confirmNextSeraiKeyCall::new((abi::Signature::from(sig),)).abi_encode().into(), - gas_limit: Self::CONFIRM_NEXT_SERAI_KEY_GAS * 120 / 100, + gas_limit: Self::CONFIRM_NEXT_SERAI_KEY_GAS * Self::GAS_REPRICING_BUFFER / 100, ..Default::default() } } @@ -351,7 +371,7 @@ impl Router { )) .abi_encode() .into(), - gas_limit: Self::UPDATE_SERAI_KEY_GAS * 120 / 100, + gas_limit: Self::UPDATE_SERAI_KEY_GAS * Self::GAS_REPRICING_BUFFER / 100, ..Default::default() } } @@ -404,18 +424,103 @@ impl Router { .abi_encode() } + /// The estimated gas cost for this OutInstruction. + /// + /// This is not guaranteed to be correct or even sufficient. It is a hint and a hint alone used + /// for determining relayer fees. + fn execute_out_instruction_gas_estimate_internal( + coin: Coin, + instruction: &abi::OutInstruction, + ) -> u64 { + // The assigned cost for performing an additional iteration of the loop + const ITERATION_COST: u64 = 5_000; + // The additional cost for a `DestinationType.Code`, as an additional buffer for its complexity + const CODE_COST: u64 = 10_000; + + let size = u64::try_from(instruction.abi_encoded_size()).unwrap(); + let calldata_memory_cost = + (NON_ZERO_BYTE_GAS_COST * size) + (MEMORY_EXPANSION_COST * size.div_ceil(32)); + + ITERATION_COST + + (match coin { + Coin::Ether => match instruction.destinationType { + // We assume we're tranferring a positive value to a cold, empty account + abi::DestinationType::Address => { + calldata_memory_cost + COLD_COST + POSITIVE_VALUE_COST + EMPTY_ACCOUNT_COST + } + abi::DestinationType::Code => { + // OutInstructions can't be encoded/decoded and doesn't have pub internals, enabling it + // to be correct by construction + let code = abi::CodeDestination::abi_decode(&instruction.destination, true).unwrap(); + // This performs a call to self with the value, incurring the positive-value cost before + // CREATE's + calldata_memory_cost + + CODE_COST + + (WARM_COST + POSITIVE_VALUE_COST + u64::from(code.gasLimit)) + } + abi::DestinationType::__Invalid => unreachable!(), + }, + Coin::Erc20(_) => { + // The ERC20 is warmed by the fee payment to the relayer + let erc20_call_gas = WARM_COST + Self::GAS_FOR_ERC20_CALL; + match instruction.destinationType { + abi::DestinationType::Address => calldata_memory_cost + erc20_call_gas, + abi::DestinationType::Code => { + let code = abi::CodeDestination::abi_decode(&instruction.destination, true).unwrap(); + calldata_memory_cost + + CODE_COST + + erc20_call_gas + + // Call to self to deploy the contract + (WARM_COST + u64::from(code.gasLimit)) + } + abi::DestinationType::__Invalid => unreachable!(), + } + } + }) + } + + /// The estimated gas cost for this OutInstruction. + /// + /// This is not guaranteed to be correct or even sufficient. It is a hint and a hint alone used + /// for determining relayer fees. + pub fn execute_out_instruction_gas_estimate(coin: Coin, address: SeraiAddress) -> u64 { + Self::execute_out_instruction_gas_estimate_internal( + coin, + &abi::OutInstruction::from(&(address, U256::ZERO)), + ) + } + + /// The estimated gas cost for this batch. + /// + /// This is not guaranteed to be correct or even sufficient. It is a hint and a hint alone used + /// for determining relayer fees. + pub fn execute_gas_estimate(coin: Coin, outs: &OutInstructions) -> u64 { + Self::EXECUTE_BASE_GAS + + (match coin { + // This is warm as it's the message sender who is called with the fee payment + Coin::Ether => WARM_COST + POSITIVE_VALUE_COST, + // This is cold as we say the fee payment is the one warming the ERC20 + Coin::Erc20(_) => COLD_COST + Self::GAS_FOR_ERC20_CALL, + }) + + outs + .0 + .iter() + .map(|out| Self::execute_out_instruction_gas_estimate_internal(coin, out)) + .sum::() + } + /// Construct a transaction to execute a batch of `OutInstruction`s. /// - /// The gas limit and gas price are not set and are left to the caller. + /// The gas limit is set to an estimate which may or may not be sufficient. The caller is + /// expected to set a correct gas limit. The gas price is not set and is left to the caller. pub fn execute(&self, coin: Coin, fee: U256, outs: OutInstructions, sig: &Signature) -> TxLegacy { - // TODO - let gas_limit = Self::EXECUTE_BASE_GAS + outs.0.iter().map(|_| 200_000 + 10_000).sum::(); + let gas = Self::execute_gas_estimate(coin, &outs); TxLegacy { to: TxKind::Call(self.address), input: abi::executeCall::new((abi::Signature::from(sig), Address::from(coin), fee, outs.0)) .abi_encode() .into(), - gas_limit: gas_limit * 120 / 100, + gas_limit: gas * Self::GAS_REPRICING_BUFFER / 100, ..Default::default() } } @@ -436,7 +541,7 @@ impl Router { TxLegacy { to: TxKind::Call(self.address), input: abi::escapeHatchCall::new((abi::Signature::from(sig), escape_to)).abi_encode().into(), - gas_limit: Self::ESCAPE_HATCH_GAS * 120 / 100, + gas_limit: Self::ESCAPE_HATCH_GAS * Self::GAS_REPRICING_BUFFER / 100, ..Default::default() } } @@ -679,6 +784,24 @@ impl Router { TransportErrorKind::Custom(format!("failed to convert nonce to u64: {e:?}").into()) })?, message_hash: event.messageHash.into(), + results: { + let results_len = usize::try_from(event.resultsLength).map_err(|e| { + TransportErrorKind::Custom( + format!("failed to convert resultsLength to usize: {e:?}").into(), + ) + })?; + if results_len.div_ceil(8) != event.results.len() { + Err(TransportErrorKind::Custom( + "resultsLength didn't align with results length".to_string().into(), + ))?; + } + let mut results = Vec::with_capacity(results_len); + for b in 0 .. results_len { + let byte = event.results[b / 8]; + results.push(((byte >> (b % 8)) & 1) == 1); + } + results + }, }); } Some(&EscapeHatchEvent::SIGNATURE_HASH) => { diff --git a/processor/ethereum/router/src/tests/erc20.rs b/processor/ethereum/router/src/tests/erc20.rs index e107fbb1..7f07f935 100644 --- a/processor/ethereum/router/src/tests/erc20.rs +++ b/processor/ethereum/router/src/tests/erc20.rs @@ -22,8 +22,10 @@ pub struct Erc20(Address); impl Erc20 { pub(crate) async fn deploy(test: &Test) -> Self { const BYTECODE: &[u8] = { - const BYTECODE_HEX: &[u8] = - include_bytes!(concat!(env!("OUT_DIR"), "/serai-processor-ethereum-router/TestERC20.bin")); + const BYTECODE_HEX: &[u8] = include_bytes!(concat!( + env!("OUT_DIR"), + "/serai-processor-ethereum-router/tests/TestERC20.bin" + )); const BYTECODE: [u8; BYTECODE_HEX.len() / 2] = match hex::const_decode_to_array::<{ BYTECODE_HEX.len() / 2 }>(BYTECODE_HEX) { Ok(bytecode) => bytecode, diff --git a/processor/ethereum/router/src/tests/mod.rs b/processor/ethereum/router/src/tests/mod.rs index 5b9748b3..8d8930ab 100644 --- a/processor/ethereum/router/src/tests/mod.rs +++ b/processor/ethereum/router/src/tests/mod.rs @@ -322,7 +322,7 @@ impl Test { coin: Coin, fee: U256, out_instructions: &[(SeraiEthereumAddress, U256)], - ) -> TxLegacy { + ) -> ([u8; 32], TxLegacy) { let out_instructions = OutInstructions::from(out_instructions); let msg = Router::execute_message( self.chain_id, @@ -331,8 +331,47 @@ impl Test { fee, out_instructions.clone(), ); + let msg_hash = ethereum_primitives::keccak256(&msg); let sig = sign(self.state.key.unwrap(), &msg); - self.router.execute(coin, fee, out_instructions, &sig) + + let mut tx = self.router.execute(coin, fee, out_instructions, &sig); + // Restore the original estimate as the gas limit to ensure it's sufficient, at least in our + // test cases + tx.gas_limit = (tx.gas_limit * 100) / Router::GAS_REPRICING_BUFFER; + + (msg_hash, tx) + } + + async fn execute( + &mut self, + coin: Coin, + fee: U256, + out_instructions: &[(SeraiEthereumAddress, U256)], + results: Vec, + ) -> u64 { + let (message_hash, mut tx) = self.execute_tx(coin, fee, out_instructions); + tx.gas_price = 100_000_000_000; + let tx = ethereum_primitives::deterministically_sign(tx); + let receipt = ethereum_test_primitives::publish_tx(&self.provider, tx.clone()).await; + assert!(receipt.status()); + // We don't check the gas for `execute` as it's infeasible. Due to our use of account + // abstraction, it isn't a critical if we do ever under-estimate, solely an unprofitable relay + + { + let block = receipt.block_number.unwrap(); + let executed = self.router.executed(block ..= block).await.unwrap(); + assert_eq!(executed.len(), 1); + assert_eq!( + executed[0], + Executed::Batch { nonce: self.state.next_nonce, message_hash, results } + ); + } + + self.state.next_nonce += 1; + self.verify_state().await; + + // We do return the gas used in case a caller can benefit from it + CalldataAgnosticGas::calculate(tx.tx(), receipt.gas_used) } fn escape_hatch_tx(&self, escape_to: Address) -> TxLegacy { @@ -402,7 +441,7 @@ async fn test_no_serai_key() { IRouterErrors::SeraiKeyWasNone(IRouter::SeraiKeyWasNone {}) )); assert!(matches!( - test.call_and_decode_err(test.execute_tx(Coin::Ether, U256::from(0), &[])).await, + test.call_and_decode_err(test.execute_tx(Coin::Ether, U256::from(0), &[]).1).await, IRouterErrors::SeraiKeyWasNone(IRouter::SeraiKeyWasNone {}) )); assert!(matches!( @@ -555,7 +594,7 @@ async fn test_erc20_router_in_instruction() { let tx = TxLegacy { chain_id: None, nonce: 0, - gas_price: 100_000_000_000u128, + gas_price: 100_000_000_000, gas_limit: 1_000_000, to: test.router.address().into(), value: U256::ZERO, @@ -592,7 +631,7 @@ async fn test_erc20_top_level_transfer_in_instruction() { let shorthand = Test::in_instruction(); let mut tx = test.router.in_instruction(coin, amount, &shorthand); - tx.gas_price = 100_000_000_000u128; + tx.gas_price = 100_000_000_000; tx.gas_limit = 1_000_000; let tx = ethereum_primitives::deterministically_sign(tx); @@ -600,6 +639,21 @@ async fn test_erc20_top_level_transfer_in_instruction() { test.publish_in_instruction_tx(tx, coin, amount, &shorthand).await; } +#[tokio::test] +async fn test_empty_execute() { + let mut test = Test::new().await; + test.confirm_next_serai_key().await; + let () = + test.provider.raw_request("anvil_setBalance".into(), (test.router.address(), 1)).await.unwrap(); + let gas_used = test.execute(Coin::Ether, U256::from(1), &[], vec![]).await; + + // For the empty ETH case, we do compare this cost to the base cost + const CALL_GAS_STIPEND: u64 = 2_300; + // We don't use the call gas stipend here + const UNUSED_GAS: u64 = CALL_GAS_STIPEND; + assert_eq!(gas_used + UNUSED_GAS, Router::EXECUTE_BASE_GAS); +} + #[tokio::test] async fn test_eth_address_out_instruction() { todo!("TODO") @@ -643,7 +697,7 @@ async fn test_escape_hatch() { let tx = ethereum_primitives::deterministically_sign(TxLegacy { to: Address([1; 20].into()).into(), gas_limit: 21_000, - gas_price: 100_000_000_000u128, + gas_price: 100_000_000_000, value: U256::from(1), ..Default::default() }); @@ -679,7 +733,7 @@ async fn test_escape_hatch() { IRouterErrors::EscapeHatchInvoked(IRouter::EscapeHatchInvoked {}) )); assert!(matches!( - test.call_and_decode_err(test.execute_tx(Coin::Ether, U256::from(0), &[])).await, + test.call_and_decode_err(test.execute_tx(Coin::Ether, U256::from(0), &[]).1).await, IRouterErrors::EscapeHatchInvoked(IRouter::EscapeHatchInvoked {}) )); // We reject further attempts to update the escape hatch to prevent the last key from being diff --git a/processor/ethereum/src/primitives/block.rs b/processor/ethereum/src/primitives/block.rs index b01493f0..9d4a8a2d 100644 --- a/processor/ethereum/src/primitives/block.rs +++ b/processor/ethereum/src/primitives/block.rs @@ -97,12 +97,19 @@ impl primitives::Block for FullEpoch { > { let mut res = HashMap::new(); for executed in &self.executed { - let Some(expected) = + let Some(mut expected) = eventualities.active_eventualities.remove(executed.nonce().to_le_bytes().as_slice()) else { // TODO: Why is this a continue, not an assert? continue; }; + // If this is a Batch Eventuality, we didn't know how the OutInstructions would resolve at + // time of creation. Copy the results from the actual transaction into the expectation + if let (Executed::Batch { results, .. }, Executed::Batch { results: expected_results, .. }) = + (executed, &mut expected.0) + { + *expected_results = results.clone(); + } assert_eq!( executed, &expected.0, @@ -119,7 +126,7 @@ impl primitives::Block for FullEpoch { Accordingly, we have free reign as to what to set the transaction ID to. We set the ID to the nonce as it's the most helpful value and unique barring someone - finding the premise for this as a hash. + finding the preimage for this as a hash. */ let mut tx_id = [0; 32]; tx_id[.. 8].copy_from_slice(executed.nonce().to_le_bytes().as_slice()); diff --git a/processor/ethereum/src/primitives/transaction.rs b/processor/ethereum/src/primitives/transaction.rs index dc430f29..a698fdb4 100644 --- a/processor/ethereum/src/primitives/transaction.rs +++ b/processor/ethereum/src/primitives/transaction.rs @@ -52,7 +52,7 @@ impl Action { Executed::NextSeraiKeySet { nonce: *nonce, key: key.eth_repr() } } Self::Batch { chain_id: _, nonce, .. } => { - Executed::Batch { nonce: *nonce, message_hash: keccak256(self.message()) } + Executed::Batch { nonce: *nonce, message_hash: keccak256(self.message()), results: vec![] } } }) } diff --git a/substrate/client/src/networks/ethereum.rs b/substrate/client/src/networks/ethereum.rs index 47b58af5..7e94dfb8 100644 --- a/substrate/client/src/networks/ethereum.rs +++ b/substrate/client/src/networks/ethereum.rs @@ -14,8 +14,8 @@ pub const ADDRESS_GAS_LIMIT: u32 = 950_000; pub struct ContractDeployment { /// The gas limit to use for this contract's execution. /// - /// THis MUST be less than the Serai gas limit. The cost of it will be deducted from the amount - /// transferred. + /// This MUST be less than the Serai gas limit. The cost of it, and the associated costs with + /// making this transaction, will be deducted from the amount transferred. gas_limit: u32, /// The initialization code of the contract to deploy. ///