diff --git a/processor/ethereum/router/contracts/Router.sol b/processor/ethereum/router/contracts/Router.sol index bc0debde..a74f8257 100644 --- a/processor/ethereum/router/contracts/Router.sol +++ b/processor/ethereum/router/contracts/Router.sol @@ -27,14 +27,13 @@ contract Router { } struct CodeDestination { - uint32 gas; + uint32 gas_limit; bytes code; } struct OutInstruction { DestinationType destinationType; bytes destination; - address coin; uint256 value; } @@ -79,7 +78,8 @@ contract Router { { // This DST needs a length prefix as well to prevent DSTs potentially being substrings of each // other, yet this fine for our very well-defined, limited use - bytes32 message = keccak256(abi.encodePacked("updateSeraiKey", block.chainid, _nonce, newSeraiKey)); + bytes32 message = + keccak256(abi.encodePacked("updateSeraiKey", block.chainid, _nonce, newSeraiKey)); _nonce++; if (!Schnorr.verify(_seraiKey, message, signature.c, signature.s)) { @@ -89,9 +89,7 @@ contract Router { function inInstruction(address coin, uint256 amount, bytes memory instruction) external payable { if (coin == address(0)) { - if (amount != msg.value) { - revert InvalidAmount(); - } + if (amount != msg.value) revert InvalidAmount(); } else { (bool success, bytes memory res) = address(coin).call( abi.encodeWithSelector(IERC20.transferFrom.selector, msg.sender, address(this), amount) @@ -100,32 +98,30 @@ contract Router { // Require there was nothing returned, which is done by some non-standard tokens, or that the // ERC20 contract did in fact return true bool nonStandardResOrTrue = (res.length == 0) || abi.decode(res, (bool)); - if (!(success && nonStandardResOrTrue)) { - revert FailedTransfer(); - } + if (!(success && nonStandardResOrTrue)) revert FailedTransfer(); } /* - Due to fee-on-transfer tokens, emitting the amount directly is frowned upon. The amount - instructed to be transferred may not actually be the amount transferred. + Due to fee-on-transfer tokens, emitting the amount directly is frowned upon. The amount + instructed to be transferred may not actually be the amount transferred. - If we add nonReentrant to every single function which can effect the balance, we can check the - amount exactly matches. This prevents transfers of less value than expected occurring, at - least, not without an additional transfer to top up the difference (which isn't routed through - this contract and accordingly isn't trying to artificially create events from this contract). + If we add nonReentrant to every single function which can effect the balance, we can check the + amount exactly matches. This prevents transfers of less value than expected occurring, at + least, not without an additional transfer to top up the difference (which isn't routed through + this contract and accordingly isn't trying to artificially create events from this contract). - If we don't add nonReentrant, a transfer can be started, and then a new transfer for the - difference can follow it up (again and again until a rounding error is reached). This contract - would believe all transfers were done in full, despite each only being done in part (except - for the last one). + If we don't add nonReentrant, a transfer can be started, and then a new transfer for the + difference can follow it up (again and again until a rounding error is reached). This contract + would believe all transfers were done in full, despite each only being done in part (except + for the last one). - Given fee-on-transfer tokens aren't intended to be supported, the only token actively planned - to be supported is Dai and it doesn't have any fee-on-transfer logic, and how fee-on-transfer - tokens aren't even able to be supported at this time by the larger Serai network, we simply - classify this entire class of tokens as non-standard implementations which induce undefined - behavior. + Given fee-on-transfer tokens aren't intended to be supported, the only token actively planned + to be supported is Dai and it doesn't have any fee-on-transfer logic, and how fee-on-transfer + tokens aren't even able to be supported at this time by the larger Serai network, we simply + classify this entire class of tokens as non-standard implementations which induce undefined + behavior. - It is the Serai network's role not to add support for any non-standard implementations. + It is the Serai network's role not to add support for any non-standard implementations. */ emit InInstruction(msg.sender, coin, amount, instruction); } @@ -133,13 +129,13 @@ contract Router { // Perform a transfer out function _transferOut(address to, address coin, uint256 value) private { /* - We on purposely do not check if these calls succeed. A call either succeeded, and there's no - problem, or the call failed due to: - A) An insolvency - B) A malicious receiver - C) A non-standard token - A is an invariant, B should be dropped, C is something out of the control of this contract. - It is again the Serai's network role to not add support for any non-standard tokens, + We on purposely do not check if these calls succeed. A call either succeeded, and there's no + problem, or the call failed due to: + A) An insolvency + B) A malicious receiver + C) A non-standard token + A is an invariant, B should be dropped, C is something out of the control of this contract. + It is again the Serai's network role to not add support for any non-standard tokens, */ if (coin == address(0)) { // Enough gas to service the transfer and a minimal amount of logic @@ -151,9 +147,9 @@ contract Router { } /* - Serai supports arbitrary calls out via deploying smart contracts (with user-specified code), - letting them execute whatever calls they're coded for. Since we can't meter CREATE, we call - CREATE from this function which we call not internally, but with CALL (which we can meter). + Serai supports arbitrary calls out via deploying smart contracts (with user-specified code), + letting them execute whatever calls they're coded for. Since we can't meter CREATE, we call + CREATE from this function which we call not internally, but with CALL (which we can meter). */ function arbitaryCallOut(bytes memory code) external { // Because we're creating a contract, increment our nonce @@ -166,12 +162,20 @@ contract Router { } // Execute a list of transactions if they were signed by the current key with the current nonce - function execute(OutInstruction[] calldata transactions, Signature calldata signature) external { + function execute( + address coin, + uint256 fee_per_gas, + 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, transactions)); + bytes32 message = + keccak256(abi.encode("execute", block.chainid, _nonce, coin, fee_per_gas, transactions)); if (!Schnorr.verify(_seraiKey, message, signature.c, signature.s)) { revert InvalidSignature(); } @@ -187,8 +191,9 @@ contract Router { if (transactions[i].destinationType == DestinationType.Address) { // This may cause a panic and the contract to become stuck if the destination isn't actually // 20 bytes. Serai is trusted to not pass a malformed destination - (AddressDestination memory destination) = abi.decode(transactions[i].destination, (AddressDestination)); - _transferOut(destination.destination, transactions[i].coin, transactions[i].value); + (AddressDestination memory destination) = + abi.decode(transactions[i].destination, (AddressDestination)); + _transferOut(destination.destination, coin, transactions[i].value); } else { // The destination is a piece of initcode. We calculate the hash of the will-be contract, // transfer to it, and then run the initcode @@ -196,15 +201,36 @@ contract Router { address(uint160(uint256(keccak256(abi.encode(address(this), _smartContractNonce))))); // Perform the transfer - _transferOut(nextAddress, transactions[i].coin, transactions[i].value); + _transferOut(nextAddress, coin, transactions[i].value); // Perform the calls with a set gas budget - (CodeDestination memory destination) = abi.decode(transactions[i].destination, (CodeDestination)); - address(this).call{ gas: destination.gas }( + (CodeDestination memory destination) = + abi.decode(transactions[i].destination, (CodeDestination)); + address(this).call{ gas: destination.gas_limit }( abi.encodeWithSelector(Router.arbitaryCallOut.selector, destination.code) ); } } + + // 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); } function nonce() external view returns (uint256) { diff --git a/processor/ethereum/router/src/lib.rs b/processor/ethereum/router/src/lib.rs index d56c514f..32fcc449 100644 --- a/processor/ethereum/router/src/lib.rs +++ b/processor/ethereum/router/src/lib.rs @@ -47,7 +47,7 @@ impl From<&Signature> for abi::Signature { } /// A coin on Ethereum. -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum Coin { /// Ether, the native coin of Ethereum. Ether, @@ -56,6 +56,14 @@ pub enum Coin { } impl Coin { + fn address(&self) -> Address { + (match self { + Coin::Ether => [0; 20], + Coin::Erc20(address) => *address, + }) + .into() + } + /// Read a `Coin`. pub fn read(reader: &mut R) -> io::Result { let mut kind = [0xff]; @@ -152,12 +160,12 @@ impl InInstruction { /// A list of `OutInstruction`s. #[derive(Clone)] pub struct OutInstructions(Vec); -impl From<&[(SeraiAddress, (Coin, U256))]> for OutInstructions { - fn from(outs: &[(SeraiAddress, (Coin, U256))]) -> Self { +impl From<&[(SeraiAddress, U256)]> for OutInstructions { + fn from(outs: &[(SeraiAddress, U256)]) -> Self { Self( outs .iter() - .map(|(address, (coin, amount))| { + .map(|(address, amount)| { #[allow(non_snake_case)] let (destinationType, destination) = match address { SeraiAddress::Address(address) => ( @@ -166,19 +174,14 @@ impl From<&[(SeraiAddress, (Coin, U256))]> for OutInstructions { ), SeraiAddress::Contract(contract) => ( abi::DestinationType::Code, - (abi::CodeDestination { gas: contract.gas(), code: contract.code().to_vec().into() }) - .abi_encode(), + (abi::CodeDestination { + gas_limit: contract.gas_limit(), + code: contract.code().to_vec().into(), + }) + .abi_encode(), ), }; - abi::OutInstruction { - destinationType, - destination: destination.into(), - coin: match coin { - Coin::Ether => [0; 20].into(), - Coin::Erc20(address) => address.into(), - }, - value: *amount, - } + abi::OutInstruction { destinationType, destination: destination.into(), value: *amount } }) .collect(), ) @@ -318,17 +321,31 @@ impl Router { } /// Get the message to be signed in order to execute a series of `OutInstruction`s. - pub fn execute_message(chain_id: U256, nonce: u64, outs: OutInstructions) -> Vec { - ("execute", chain_id, U256::try_from(nonce).expect("couldn't convert u64 to u256"), outs.0) + pub fn execute_message( + chain_id: U256, + nonce: u64, + coin: Coin, + fee_per_gas: U256, + outs: OutInstructions, + ) -> Vec { + ("execute", chain_id, U256::try_from(nonce).unwrap(), coin.address(), fee_per_gas, outs.0) .abi_encode() } /// Construct a transaction to execute a batch of `OutInstruction`s. - pub fn execute(&self, outs: OutInstructions, sig: &Signature) -> TxLegacy { + pub fn execute( + &self, + coin: Coin, + fee_per_gas: U256, + outs: OutInstructions, + sig: &Signature, + ) -> TxLegacy { let outs_len = outs.0.len(); TxLegacy { to: TxKind::Call(self.1), - input: abi::executeCall::new((outs.0, sig.into())).abi_encode().into(), + input: abi::executeCall::new((coin.address(), fee_per_gas, outs.0, sig.into())) + .abi_encode() + .into(), // TODO gas_limit: 100_000 + ((200_000 + 10_000) * u128::try_from(outs_len).unwrap()), ..Default::default()