From bc1bbf99514728252c3f18080153a59b6a53b319 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 20 Sep 2024 00:20:05 -0400 Subject: [PATCH] Set a fixed fee transferred to the caller for publication Avoids the risk of the gas used by the contract exceeding the gas presumed to be used (causing an insolvency). --- .../ethereum/router/contracts/Router.sol | 25 +++---------------- processor/ethereum/router/src/lib.rs | 17 +++---------- .../ethereum/src/primitives/transaction.rs | 25 +++++++++---------- processor/ethereum/src/publisher.rs | 4 +-- processor/ethereum/src/scheduler.rs | 11 +++++--- 5 files changed, 28 insertions(+), 54 deletions(-) diff --git a/processor/ethereum/router/contracts/Router.sol b/processor/ethereum/router/contracts/Router.sol index a74f8257..c4c038e2 100644 --- a/processor/ethereum/router/contracts/Router.sol +++ b/processor/ethereum/router/contracts/Router.sol @@ -164,18 +164,16 @@ contract Router { // Execute a list of transactions if they were signed by the current key with the current nonce function execute( address coin, - uint256 fee_per_gas, + uint256 fee, OutInstruction[] calldata transactions, Signature calldata signature ) external { - uint256 gasLeftAtStart = gasleft(); - // Verify the signature // We hash the message here as we need the message's hash for the Executed event // Since we're already going to hash it, hashing it prior to verifying the signature reduces the // amount of words hashed by its challenge function (reducing our gas costs) bytes32 message = - keccak256(abi.encode("execute", block.chainid, _nonce, coin, fee_per_gas, transactions)); + keccak256(abi.encode("execute", block.chainid, _nonce, coin, fee, transactions)); if (!Schnorr.verify(_seraiKey, message, signature.c, signature.s)) { revert InvalidSignature(); } @@ -212,25 +210,8 @@ contract Router { } } - // Calculate the gas which will be used to transfer the fee out - // This is meant to be always over, never under, with any excess being a tip to the publisher - uint256 gasToTransferOut; - if (coin == address(0)) { - // 5,000 gas is explicitly allowed, with another 10,000 for whatever overhead remains - // unaccounted for - gasToTransferOut = 15_000; - } else { - // 100_000 gas is explicitly allowed, with another 15,000 for whatever overhead remains - // unaccounted for. More gas is given than for ETH due to needing to ABI encode the function - // call - gasToTransferOut = 115_000; - } - - // Calculate the gas used - uint256 gasLeftAtEnd = gasleft(); - uint256 gasUsed = gasLeftAtStart - gasLeftAtEnd; // Transfer to the caller the fee - _transferOut(msg.sender, coin, (gasUsed + gasToTransferOut) * fee_per_gas); + _transferOut(msg.sender, coin, fee); } function nonce() external view returns (uint256) { diff --git a/processor/ethereum/router/src/lib.rs b/processor/ethereum/router/src/lib.rs index 32fcc449..248523b8 100644 --- a/processor/ethereum/router/src/lib.rs +++ b/processor/ethereum/router/src/lib.rs @@ -325,27 +325,18 @@ impl Router { chain_id: U256, nonce: u64, coin: Coin, - fee_per_gas: U256, + fee: U256, outs: OutInstructions, ) -> Vec { - ("execute", chain_id, U256::try_from(nonce).unwrap(), coin.address(), fee_per_gas, outs.0) - .abi_encode() + ("execute", chain_id, U256::try_from(nonce).unwrap(), coin.address(), fee, outs.0).abi_encode() } /// Construct a transaction to execute a batch of `OutInstruction`s. - pub fn execute( - &self, - coin: Coin, - fee_per_gas: U256, - outs: OutInstructions, - sig: &Signature, - ) -> TxLegacy { + pub fn execute(&self, coin: Coin, fee: U256, outs: OutInstructions, sig: &Signature) -> TxLegacy { let outs_len = outs.0.len(); TxLegacy { to: TxKind::Call(self.1), - input: abi::executeCall::new((coin.address(), fee_per_gas, outs.0, sig.into())) - .abi_encode() - .into(), + input: abi::executeCall::new((coin.address(), fee, outs.0, sig.into())).abi_encode().into(), // TODO gas_limit: 100_000 + ((200_000 + 10_000) * u128::try_from(outs_len).unwrap()), ..Default::default() diff --git a/processor/ethereum/src/primitives/transaction.rs b/processor/ethereum/src/primitives/transaction.rs index 67f17d31..6730e7a9 100644 --- a/processor/ethereum/src/primitives/transaction.rs +++ b/processor/ethereum/src/primitives/transaction.rs @@ -18,7 +18,7 @@ use crate::{output::OutputId, machine::ClonableTransctionMachine}; #[derive(Clone, PartialEq, Debug)] pub(crate) enum Action { SetKey { chain_id: U256, nonce: u64, key: PublicKey }, - Batch { chain_id: U256, nonce: u64, coin: Coin, fee_per_gas: U256, outs: Vec<(Address, U256)> }, + Batch { chain_id: U256, nonce: u64, coin: Coin, fee: U256, outs: Vec<(Address, U256)> }, } #[derive(Clone, PartialEq, Eq, Debug)] @@ -36,11 +36,11 @@ impl Action { Action::SetKey { chain_id, nonce, key } => { Router::update_serai_key_message(*chain_id, *nonce, key) } - Action::Batch { chain_id, nonce, coin, fee_per_gas, outs } => Router::execute_message( + Action::Batch { chain_id, nonce, coin, fee, outs } => Router::execute_message( *chain_id, *nonce, *coin, - *fee_per_gas, + *fee, OutInstructions::from(outs.as_ref()), ), } @@ -51,10 +51,9 @@ impl Action { Self::SetKey { chain_id: _, nonce, key } => { Executed::SetKey { nonce: *nonce, key: key.eth_repr() } } - Self::Batch { nonce, .. } => Executed::Batch { - nonce: *nonce, - message_hash: keccak256(self.message()), - }, + Self::Batch { nonce, .. } => { + Executed::Batch { nonce: *nonce, message_hash: keccak256(self.message()) } + } }) } } @@ -106,9 +105,9 @@ impl SignableTransaction for Action { 1 => { let coin = Coin::read(reader)?; - let mut fee_per_gas = [0; 32]; - reader.read_exact(&mut fee_per_gas)?; - let fee_per_gas = U256::from_le_bytes(fee_per_gas); + let mut fee = [0; 32]; + reader.read_exact(&mut fee)?; + let fee = U256::from_le_bytes(fee); let mut outs_len = [0; 4]; reader.read_exact(&mut outs_len)?; @@ -124,7 +123,7 @@ impl SignableTransaction for Action { outs.push((address, amount)); } - Action::Batch { chain_id, nonce, coin, fee_per_gas, outs } + Action::Batch { chain_id, nonce, coin, fee, outs } } _ => unreachable!(), }) @@ -137,12 +136,12 @@ impl SignableTransaction for Action { writer.write_all(&nonce.to_le_bytes())?; writer.write_all(&key.eth_repr()) } - Self::Batch { chain_id, nonce, coin, fee_per_gas, outs } => { + Self::Batch { chain_id, nonce, coin, fee, outs } => { writer.write_all(&[1])?; writer.write_all(&chain_id.as_le_bytes())?; writer.write_all(&nonce.to_le_bytes())?; coin.write(writer)?; - writer.write_all(&fee_per_gas.as_le_bytes())?; + writer.write_all(&fee.as_le_bytes())?; writer.write_all(&u32::try_from(outs.len()).unwrap().to_le_bytes())?; for (address, amount) in outs { borsh::BorshSerialize::serialize(address, writer)?; diff --git a/processor/ethereum/src/publisher.rs b/processor/ethereum/src/publisher.rs index a49ea67f..3d18a6ef 100644 --- a/processor/ethereum/src/publisher.rs +++ b/processor/ethereum/src/publisher.rs @@ -89,8 +89,8 @@ impl signers::TransactionPublisher for TransactionPublisher< // Convert from an Action (an internal representation of a signable event) to a TxLegacy let tx = match tx.0 { Action::SetKey { chain_id: _, nonce: _, key } => router.update_serai_key(&key, &tx.1), - Action::Batch { chain_id: _, nonce: _, coin, fee_per_gas, outs } => { - router.execute(coin, fee_per_gas, OutInstructions::from(outs.as_ref()), &tx.1) + Action::Batch { chain_id: _, nonce: _, coin, fee, outs } => { + router.execute(coin, fee, OutInstructions::from(outs.as_ref()), &tx.1) } }; diff --git a/processor/ethereum/src/scheduler.rs b/processor/ethereum/src/scheduler.rs index 5a3fd428..f4c31ec6 100644 --- a/processor/ethereum/src/scheduler.rs +++ b/processor/ethereum/src/scheduler.rs @@ -111,16 +111,19 @@ impl smart_contract_scheduler::SmartContract> for SmartContract { // Push each batch onto the result for outs in batches { - let base_gas = BASE_GAS.div_ceil(u32::try_from(outs.len()).unwrap()); + let mut total_gas = 0; + + let base_gas_per_payment = BASE_GAS.div_ceil(u32::try_from(outs.len()).unwrap()); // Deduce the fee from each out for out in &mut outs { - let payment_gas = base_gas + + let payment_gas = base_gas_per_payment + match out.0 { Address::Address(_) => ADDRESS_PAYMENT_GAS, Address::Contract(deployment) => CONTRACT_PAYMENT_GAS + deployment.gas_limit(), }; + total_gas += payment_gas; - let payment_gas_cost = fee_per_gas * U256::try_from(payment_gas).unwrap(); + let payment_gas_cost = U256::try_from(payment_gas).unwrap() * fee_per_gas; out.1 -= payment_gas_cost; } @@ -128,7 +131,7 @@ impl smart_contract_scheduler::SmartContract> for SmartContract { chain_id: self.chain_id, nonce, coin: coin_to_ethereum_coin(coin), - fee_per_gas, + fee: U256::try_from(total_gas).unwrap() * fee_per_gas, outs, }); nonce += 1;