diff --git a/.github/actions/build-dependencies/action.yml b/.github/actions/build-dependencies/action.yml index 5994b723..47d77522 100644 --- a/.github/actions/build-dependencies/action.yml +++ b/.github/actions/build-dependencies/action.yml @@ -42,8 +42,8 @@ runs: shell: bash run: | cargo install svm-rs - svm install 0.8.25 - svm use 0.8.25 + svm install 0.8.26 + svm use 0.8.26 # - name: Cache Rust # uses: Swatinem/rust-cache@a95ba195448af2da9b00fb742d14ffaaf3c21f43 diff --git a/networks/ethereum/build-contracts/src/lib.rs b/networks/ethereum/build-contracts/src/lib.rs index 4fee315a..5213059e 100644 --- a/networks/ethereum/build-contracts/src/lib.rs +++ b/networks/ethereum/build-contracts/src/lib.rs @@ -6,8 +6,12 @@ use std::{path::PathBuf, fs, process::Command}; /// Build contracts from the specified path, outputting the artifacts to the specified path. /// -/// Requires solc 0.8.25. -pub fn build(contracts_path: &str, artifacts_path: &str) -> Result<(), String> { +/// Requires solc 0.8.26. +pub fn build( + include_paths: &[&str], + contracts_path: &str, + artifacts_path: &str, +) -> Result<(), String> { println!("cargo:rerun-if-changed={contracts_path}/*"); println!("cargo:rerun-if-changed={artifacts_path}/*"); @@ -24,20 +28,24 @@ pub fn build(contracts_path: &str, artifacts_path: &str) -> Result<(), String> { if let Some(version) = line.strip_prefix("Version: ") { let version = version.split('+').next().ok_or_else(|| "no value present on line".to_string())?; - if version != "0.8.25" { - Err(format!("version was {version}, 0.8.25 required"))? + if version != "0.8.26" { + Err(format!("version was {version}, 0.8.26 required"))? } } } #[rustfmt::skip] - let args = [ + let mut args = vec![ "--base-path", ".", "-o", artifacts_path, "--overwrite", "--bin", "--bin-runtime", "--abi", "--via-ir", "--optimize", "--no-color", ]; + for include_path in include_paths { + args.push("--include-path"); + args.push(include_path); + } let mut args = args.into_iter().map(str::to_string).collect::>(); let mut queue = vec![PathBuf::from(contracts_path)]; @@ -70,17 +78,17 @@ pub fn build(contracts_path: &str, artifacts_path: &str) -> Result<(), String> { } let solc = Command::new("solc") - .args(args) + .args(args.clone()) .output() .map_err(|_| "couldn't fetch solc output".to_string())?; let stderr = String::from_utf8(solc.stderr).map_err(|_| "solc stderr wasn't UTF-8".to_string())?; if !solc.status.success() { - Err(format!("solc didn't successfully execute: {stderr}"))?; + Err(format!("solc (`{}`) didn't successfully execute: {stderr}", args.join(" ")))?; } for line in stderr.lines() { if line.contains("Error:") { - Err(format!("solc output had error: {stderr}"))?; + Err(format!("solc (`{}`) output had error: {stderr}", args.join(" ")))?; } } diff --git a/networks/ethereum/schnorr/.gitignore b/networks/ethereum/schnorr/.gitignore deleted file mode 100644 index de153db3..00000000 --- a/networks/ethereum/schnorr/.gitignore +++ /dev/null @@ -1 +0,0 @@ -artifacts diff --git a/networks/ethereum/schnorr/build.rs b/networks/ethereum/schnorr/build.rs index 300f8949..7b7c30fd 100644 --- a/networks/ethereum/schnorr/build.rs +++ b/networks/ethereum/schnorr/build.rs @@ -5,5 +5,5 @@ fn main() { if !fs::exists(&artifacts_path).unwrap() { fs::create_dir(&artifacts_path).unwrap(); } - build_solidity_contracts::build("contracts", &artifacts_path).unwrap(); + build_solidity_contracts::build(&[], "contracts", &artifacts_path).unwrap(); } diff --git a/networks/ethereum/schnorr/contracts/Schnorr.sol b/networks/ethereum/schnorr/contracts/Schnorr.sol index b13696cf..182e90e3 100644 --- a/networks/ethereum/schnorr/contracts/Schnorr.sol +++ b/networks/ethereum/schnorr/contracts/Schnorr.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-only -pragma solidity ^0.8.0; +pragma solidity ^0.8.26; // See https://github.com/noot/schnorr-verify for implementation details library Schnorr { diff --git a/networks/ethereum/schnorr/contracts/tests/Schnorr.sol b/networks/ethereum/schnorr/contracts/tests/Schnorr.sol index 18a58cf9..26be683d 100644 --- a/networks/ethereum/schnorr/contracts/tests/Schnorr.sol +++ b/networks/ethereum/schnorr/contracts/tests/Schnorr.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-only -pragma solidity ^0.8.0; +pragma solidity ^0.8.26; import "../Schnorr.sol"; diff --git a/orchestration/src/processor.rs b/orchestration/src/processor.rs index 3387c4ed..00f9243d 100644 --- a/orchestration/src/processor.rs +++ b/orchestration/src/processor.rs @@ -21,8 +21,8 @@ pub fn processor( if coin == "ethereum" { r#" RUN cargo install svm-rs -RUN svm install 0.8.25 -RUN svm use 0.8.25 +RUN svm install 0.8.26 +RUN svm use 0.8.26 "# } else { "" diff --git a/processor/ethereum/contracts/Cargo.toml b/processor/ethereum/contracts/Cargo.toml index f09eb938..64fbccad 100644 --- a/processor/ethereum/contracts/Cargo.toml +++ b/processor/ethereum/contracts/Cargo.toml @@ -17,7 +17,7 @@ rustdoc-args = ["--cfg", "docsrs"] workspace = true [dependencies] -alloy-sol-types = { version = "0.8", default-features = false } +alloy-sol-types = { version = "0.8", default-features = false, features = ["json"] } [build-dependencies] build-solidity-contracts = { path = "../../../networks/ethereum/build-contracts" } diff --git a/processor/ethereum/contracts/build.rs b/processor/ethereum/contracts/build.rs index 8e310b60..0af41608 100644 --- a/processor/ethereum/contracts/build.rs +++ b/processor/ethereum/contracts/build.rs @@ -1,3 +1,8 @@ fn main() { - build_solidity_contracts::build("contracts", "artifacts").unwrap(); + build_solidity_contracts::build( + &["../../../networks/ethereum/schnorr/contracts"], + "contracts", + "artifacts", + ) + .unwrap(); } diff --git a/processor/ethereum/contracts/contracts/Deployer.sol b/processor/ethereum/contracts/contracts/Deployer.sol index 475be4c1..1c05e38a 100644 --- a/processor/ethereum/contracts/contracts/Deployer.sol +++ b/processor/ethereum/contracts/contracts/Deployer.sol @@ -1,5 +1,5 @@ -// SPDX-License-Identifier: AGPLv3 -pragma solidity ^0.8.0; +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity ^0.8.26; /* The expected deployment process of the Router is as follows: diff --git a/processor/ethereum/contracts/contracts/IERC20.sol b/processor/ethereum/contracts/contracts/IERC20.sol index 70f1f93c..c2de5ca0 100644 --- a/processor/ethereum/contracts/contracts/IERC20.sol +++ b/processor/ethereum/contracts/contracts/IERC20.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: CC0 -pragma solidity ^0.8.0; +pragma solidity ^0.8.26; interface IERC20 { event Transfer(address indexed from, address indexed to, uint256 value); diff --git a/processor/ethereum/contracts/contracts/Router.sol b/processor/ethereum/contracts/contracts/Router.sol index c5e1efa2..65541a10 100644 --- a/processor/ethereum/contracts/contracts/Router.sol +++ b/processor/ethereum/contracts/contracts/Router.sol @@ -1,23 +1,31 @@ -// SPDX-License-Identifier: AGPLv3 -pragma solidity ^0.8.0; +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity ^0.8.26; import "./IERC20.sol"; -import "./Schnorr.sol"; -import "./Sandbox.sol"; +import "Schnorr.sol"; +// _ is used as a prefix for internal functions and smart-contract-scoped variables contract Router { - // Nonce is incremented for each batch of transactions executed/key update - uint256 public nonce; + // Nonce is incremented for each command executed, preventing replays + uint256 private _nonce; - // Current public key's x-coordinate - // This key must always have the parity defined within the Schnorr contract - bytes32 public seraiKey; + // The nonce which will be used for the smart contracts we deploy, enabling + // predicting their addresses + uint256 private _smartContractNonce; + + // The current public key, defined as per the Schnorr library + bytes32 private _seraiKey; + + enum DestinationType { + Address, + Code + } struct OutInstruction { - address to; - Call[] calls; - + DestinationType destinationType; + bytes destination; + address coin; uint256 value; } @@ -26,70 +34,42 @@ contract Router { bytes32 s; } - event SeraiKeyUpdated( - uint256 indexed nonce, - bytes32 indexed key, - Signature signature - ); - event InInstruction( - address indexed from, - address indexed coin, - uint256 amount, - bytes instruction - ); - // success is a uint256 representing a bitfield of transaction successes - event Executed( - uint256 indexed nonce, - bytes32 indexed batch, - uint256 success, - Signature signature - ); + event SeraiKeyUpdated(uint256 indexed nonce, bytes32 indexed key); + event InInstruction(address indexed from, address indexed coin, uint256 amount, bytes instruction); + event Executed(uint256 indexed nonce, bytes32 indexed batch); - // error types - error InvalidKey(); error InvalidSignature(); error InvalidAmount(); error FailedTransfer(); - error TooManyTransactions(); - - modifier _updateSeraiKeyAtEndOfFn( - uint256 _nonce, - bytes32 key, - Signature memory sig - ) { - if ( - (key == bytes32(0)) || - ((bytes32(uint256(key) % Schnorr.Q)) != key) - ) { - revert InvalidKey(); - } + // Update the Serai key at the end of the current function. + modifier _updateSeraiKeyAtEndOfFn(uint256 nonceUpdatedWith, bytes32 newSeraiKey) { + // Run the function itself. _; - seraiKey = key; - emit SeraiKeyUpdated(_nonce, key, sig); + // Update the key. + _seraiKey = newSeraiKey; + emit SeraiKeyUpdated(nonceUpdatedWith, newSeraiKey); } - constructor(bytes32 _seraiKey) _updateSeraiKeyAtEndOfFn( - 0, - _seraiKey, - Signature({ c: bytes32(0), s: bytes32(0) }) - ) { - nonce = 1; + constructor(bytes32 initialSeraiKey) _updateSeraiKeyAtEndOfFn(0, initialSeraiKey) { + // We consumed nonce 0 when setting the initial Serai key + _nonce = 1; + // Nonces are incremented by 1 upon account creation, prior to any code execution, per EIP-161 + // This is incompatible with any networks which don't have their nonces start at 0 + _smartContractNonce = 1; } - // updateSeraiKey validates the given Schnorr signature against the current - // public key, and if successful, updates the contract's public key to the - // given one. + // updateSeraiKey validates the given Schnorr signature against the current public key, and if + // successful, updates the contract's public key to the one specified. function updateSeraiKey( - bytes32 _seraiKey, - Signature calldata sig - ) external _updateSeraiKeyAtEndOfFn(nonce, _seraiKey, sig) { - bytes memory message = - abi.encodePacked("updateSeraiKey", block.chainid, nonce, _seraiKey); - nonce++; + bytes32 newSeraiKey, + Signature calldata signature + ) external _updateSeraiKeyAtEndOfFn(_nonce, newSeraiKey) { + bytes memory message = abi.encodePacked("updateSeraiKey", block.chainid, _nonce, newSeraiKey); + _nonce++; - if (!Schnorr.verify(seraiKey, message, sig.c, sig.s)) { + if (!Schnorr.verify(_seraiKey, message, signature.c, signature.s)) { revert InvalidSignature(); } } @@ -114,109 +94,121 @@ 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)); + // 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(); } } /* - Due to fee-on-transfer tokens, emitting the amount directly is frowned upon. - The amount instructed to transfer 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). + 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 planned to be supported is Dai and it doesn't have any fee-on-transfer - logic, fee-on-transfer tokens aren't even able to be supported at this time, - 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. + 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. */ emit InInstruction(msg.sender, coin, amount, instruction); } - // execute accepts a list of transactions to execute as well as a signature. - // if signature verification passes, the given transactions are executed. - // if signature verification fails, this function will revert. - function execute( - OutInstruction[] calldata transactions, - Signature calldata sig - ) external { - if (transactions.length > 256) { - revert TooManyTransactions(); + // 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, + */ + if (coin == address(0)) { + // Enough gas to service the transfer and a minimal amount of logic + to.call{ value: value, gas: 5_000 }(""); + } else { + coin.call{ gas: 100_000 }(abi.encodeWithSelector(IERC20.transfer.selector, msg.sender, value)); } + } - bytes memory message = - abi.encode("execute", block.chainid, nonce, transactions); - uint256 executed_with_nonce = nonce; - // This prevents re-entrancy from causing double spends yet does allow - // out-of-order execution via re-entrancy - nonce++; + /* + 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 + _smartContractNonce += 1; - if (!Schnorr.verify(seraiKey, message, sig.c, sig.s)) { + address contractAddress; + assembly { + contractAddress := create(0, add(code, 0x20), mload(code)) + } + } + + // 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 { + // Verify the signature + bytes memory message = abi.encode("execute", block.chainid, _nonce, transactions); + if (!Schnorr.verify(_seraiKey, message, signature.c, signature.s)) { revert InvalidSignature(); } - uint256 successes; + // Since the signature was verified, perform execution + emit Executed(_nonce, keccak256(message)); + // While this is sufficient to prevent replays, it's still technically possible for instructions + // from later batches to be executed before these instructions upon re-entrancy + _nonce++; + for (uint256 i = 0; i < transactions.length; i++) { - bool success; - - // If there are no calls, send to `to` the value - if (transactions[i].calls.length == 0) { - (success, ) = transactions[i].to.call{ - value: transactions[i].value, - gas: 5_000 - }(""); + // If the destination is an address, we perform a direct transfer + 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 + (address destination) = abi.decode(transactions[i].destination, (address)); + _transferOut(destination, transactions[i].coin, transactions[i].value); } else { - // If there are calls, ignore `to`. Deploy a new Sandbox and proxy the - // calls through that - // - // We could use a single sandbox in order to reduce gas costs, yet that - // risks one person creating an approval that's hooked before another - // user's intended action executes, in order to drain their coins - // - // While technically, that would be a flaw in the sandboxed flow, this - // is robust and prevents such flaws from being possible - // - // We also don't want people to set state via the Sandbox and expect it - // future available when anyone else could set a distinct value - Sandbox sandbox = new Sandbox(); - (success, ) = address(sandbox).call{ - value: transactions[i].value, - // TODO: Have the Call specify the gas up front - gas: 350_000 - }( - abi.encodeWithSelector( - Sandbox.sandbox.selector, - transactions[i].calls - ) - ); - } + // The destination is a piece of initcode. We calculate the hash of the will-be contract, + // transfer to it, and then run the initcode + address nextAddress = + address(uint160(uint256(keccak256(abi.encode(address(this), _smartContractNonce))))); - assembly { - successes := or(successes, shl(i, success)) + // Perform the transfer + _transferOut(nextAddress, transactions[i].coin, transactions[i].value); + + // Perform the calls with a set gas budget + (uint24 gas, bytes memory code) = abi.decode(transactions[i].destination, (uint24, bytes)); + address(this).call{ + gas: gas + }(abi.encodeWithSelector(Router.arbitaryCallOut.selector, code)); } } - emit Executed( - executed_with_nonce, - keccak256(message), - successes, - sig - ); + } + + function nonce() external view returns (uint256) { + return _nonce; + } + + function smartContractNonce() external view returns (uint256) { + return _smartContractNonce; + } + + function seraiKey() external view returns (bytes32) { + return _seraiKey; } } diff --git a/processor/ethereum/contracts/contracts/Sandbox.sol b/processor/ethereum/contracts/contracts/Sandbox.sol deleted file mode 100644 index a82a3afd..00000000 --- a/processor/ethereum/contracts/contracts/Sandbox.sol +++ /dev/null @@ -1,48 +0,0 @@ -// SPDX-License-Identifier: AGPLv3 -pragma solidity ^0.8.24; - -struct Call { - address to; - uint256 value; - bytes data; -} - -// A minimal sandbox focused on gas efficiency. -// -// The first call is executed if any of the calls fail, making it a fallback. -// All other calls are executed sequentially. -contract Sandbox { - error AlreadyCalled(); - error CallsFailed(); - - function sandbox(Call[] calldata calls) external payable { - // Prevent re-entrancy due to this executing arbitrary calls from anyone - // and anywhere - bool called; - assembly { called := tload(0) } - if (called) { - revert AlreadyCalled(); - } - assembly { tstore(0, 1) } - - // Execute the calls, starting from 1 - for (uint256 i = 1; i < calls.length; i++) { - (bool success, ) = - calls[i].to.call{ value: calls[i].value }(calls[i].data); - - // If this call failed, execute the fallback (call 0) - if (!success) { - (success, ) = - calls[0].to.call{ value: address(this).balance }(calls[0].data); - // If this call also failed, revert entirely - if (!success) { - revert CallsFailed(); - } - return; - } - } - - // We don't clear the re-entrancy guard as this contract should never be - // called again, so there's no reason to spend the effort - } -} diff --git a/processor/ethereum/contracts/contracts/tests/ERC20.sol b/processor/ethereum/contracts/contracts/tests/ERC20.sol index e157974c..f38bfea4 100644 --- a/processor/ethereum/contracts/contracts/tests/ERC20.sol +++ b/processor/ethereum/contracts/contracts/tests/ERC20.sol @@ -1,5 +1,5 @@ -// SPDX-License-Identifier: AGPLv3 -pragma solidity ^0.8.0; +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity ^0.8.26; contract TestERC20 { event Transfer(address indexed from, address indexed to, uint256 value); diff --git a/processor/ethereum/contracts/src/lib.rs b/processor/ethereum/contracts/src/lib.rs index fef10288..d8de29b3 100644 --- a/processor/ethereum/contracts/src/lib.rs +++ b/processor/ethereum/contracts/src/lib.rs @@ -44,5 +44,3 @@ pub mod router { pub const BYTECODE: &str = include_str!("../artifacts/Router.bin"); pub use super::router_container::Router::*; } - -pub mod tests; diff --git a/processor/ethereum/contracts/src/tests.rs b/processor/ethereum/contracts/src/tests.rs deleted file mode 100644 index 9f141c29..00000000 --- a/processor/ethereum/contracts/src/tests.rs +++ /dev/null @@ -1,13 +0,0 @@ -use alloy_sol_types::sol; - -#[rustfmt::skip] -#[allow(warnings)] -#[allow(needless_pass_by_value)] -#[allow(clippy::all)] -#[allow(clippy::ignored_unit_patterns)] -#[allow(clippy::redundant_closure_for_method_calls)] -mod schnorr_container { - use super::*; - sol!("contracts/tests/Schnorr.sol"); -} -pub use schnorr_container::TestSchnorr as schnorr;