Natspec, slither Deployer, Router

.github/workflows/lint.yml

@@ -90,3 +90,25 @@ jobs:
         run: |
           cargo install cargo-machete
           cargo machete
+
+  slither:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@3df4ab11eba7bda6032a0b82a6bb43b11571feac
+      - name: Slither
+        run: |
+          python3 -m pip install solc-select
+          solc-select install 0.8.26
+          solc-select use 0.8.26
+
+          python3 -m pip install slither-analyzer
+
+          slither --include-paths ./networks/ethereum/schnorr/contracts/Schnorr.sol
+          slither --include-paths ./networks/ethereum/schnorr/contracts ./networks/ethereum/schnorr/contracts/tests/Schnorr.sol
+          slither processor/ethereum/deployer/contracts/Deployer.sol
+          slither processor/ethereum/erc20/contracts/IERC20.sol
+
+          cp networks/ethereum/schnorr/contracts/Schnorr.sol processor/ethereum/router/contracts/
+          cp processor/ethereum/erc20/contracts/IERC20.sol processor/ethereum/router/contracts/
+          cd processor/ethereum/router/contracts
+          slither Router.sol

networks/ethereum/schnorr/contracts/Schnorr.sol

@@ -2,12 +2,17 @@
 pragma solidity ^0.8.26;
 
 /// @title A library for verifying Schnorr signatures
-/// @author Luke Parker <lukeparker5132@gmail.com>
+/// @author Luke Parker <lukeparker@serai.exchange>
 /// @author Elizabeth Binks <elizabethjbinks@gmail.com>
 /// @notice Verifies a Schnorr signature for a specified public key
-/// @dev This contract is not complete. Only certain public keys are compatible
+/**
-/// @dev See https://github.com/serai-dex/serai/blob/next/networks/ethereum/schnorr/src/tests/premise.rs for implementation details
+ * @dev This contract is not complete (in the cryptographic sense). Only a subset of potential
-// TODO: Pin to a specific branch/commit once `next` is merged into `develop`
+ *  public keys are representable here.
+ *
+ *   See https://github.com/serai-dex/serai/blob/next/networks/ethereum/schnorr/src/tests/premise.rs
+ *   for implementation details
+ */
+// TODO: Pin above link to a specific branch/commit once `next` is merged into `develop`
 library Schnorr {
   // secp256k1 group order
   uint256 private constant Q = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141;
@@ -18,26 +23,30 @@ library Schnorr {
   uint8 private constant KEY_PARITY = 27;
 
   /// @notice Verifies a Schnorr signature for the specified public key
-  /// @dev The y-coordinate of the public key is assumed to be even
+  /**
-  /// @dev The x-coordinate of the public key is assumed to be less than the order of secp256k1
+   * @dev The y-coordinate of the public key is assumed to be even. The x-coordinate of the public
-  /// @dev The challenge is calculated as `keccak256(abi.encodePacked(address(R), public_key, message))` where `R` is the commitment to the Schnorr signature's nonce
+   *   key is assumed to be less than the order of secp256k1.
-  /// @param public_key The x-coordinate of the public key
+   *
+   *   The challenge is calculated as `keccak256(abi.encodePacked(address(R), publicKey, message))`
+   *   where `R` is the commitment to the Schnorr signature's nonce.
+   */
+  /// @param publicKey The x-coordinate of the public key
   /// @param message The (hash of the) message signed
   /// @param c The challenge for the Schnorr signature
   /// @param s The response to the challenge for the Schnorr signature
   /// @return If the signature is valid
-  function verify(bytes32 public_key, bytes32 message, bytes32 c, bytes32 s)
+  function verify(bytes32 publicKey, bytes32 message, bytes32 c, bytes32 s)
     internal
     pure
     returns (bool)
   {
     // ecrecover = (m, v, r, s) -> key
     // We instead pass the following to recover the Schnorr signature's nonce (not a public key)
-    bytes32 sa = bytes32(Q - mulmod(uint256(s), uint256(public_key), Q));
+    bytes32 sa = bytes32(Q - mulmod(uint256(s), uint256(publicKey), Q));
-    bytes32 ca = bytes32(Q - mulmod(uint256(c), uint256(public_key), Q));
+    bytes32 ca = bytes32(Q - mulmod(uint256(c), uint256(publicKey), Q));
 
     /*
-      The ecrecover precompile checks `r` and `s` (`public_key` and `ca`) are non-zero, banning the
+      The ecrecover precompile checks `r` and `s` (`publicKey` and `ca`) are non-zero, banning the
       two keys with zero for their x-coordinate and zero challenges. Each already only had a
       negligible probability of occuring (assuming zero x-coordinates are even on-curve in the first
       place).
@@ -45,11 +54,11 @@ library Schnorr {
       `sa` is not checked to be non-zero yet it does not need to be. The inverse of it is never
       taken.
     */
-    address R = ecrecover(sa, KEY_PARITY, public_key, ca);
+    address R = ecrecover(sa, KEY_PARITY, publicKey, ca);
     // The ecrecover failed
     if (R == address(0)) return false;
 
     // Check the signature is correct by rebuilding the challenge
-    return c == keccak256(abi.encodePacked(R, public_key, message));
+    return c == keccak256(abi.encodePacked(R, publicKey, message));
   }
 }

networks/ethereum/schnorr/contracts/tests/Schnorr.sol

@@ -8,19 +8,23 @@ import "../Schnorr.sol";
 /// @author Elizabeth Binks <elizabethjbinks@gmail.com>
 contract TestSchnorr {
   /// @notice Verifies a Schnorr signature for the specified public key
-  /// @dev The y-coordinate of the public key is assumed to be even
+  /**
-  /// @dev The x-coordinate of the public key is assumed to be less than the order of secp256k1
+   * @dev The y-coordinate of the public key is assumed to be even. The x-coordinate of the public
-  /// @dev The challenge is calculated as `keccak256(abi.encodePacked(address(R), public_key, message))` where `R` is the commitment to the Schnorr signature's nonce
+   *   key is assumed to be less than the order of secp256k1.
-  /// @param public_key The x-coordinate of the public key
+   *
+   *   The challenge is calculated as `keccak256(abi.encodePacked(address(R), publicKey, message))`
+   *   where `R` is the commitment to the Schnorr signature's nonce.
+   */
+  /// @param publicKey The x-coordinate of the public key
   /// @param message The (hash of the) message signed
   /// @param c The challenge for the Schnorr signature
   /// @param s The response to the challenge for the Schnorr signature
   /// @return If the signature is valid
-  function verify(bytes32 public_key, bytes calldata message, bytes32 c, bytes32 s)
+  function verify(bytes32 publicKey, bytes calldata message, bytes32 c, bytes32 s)
     external
     pure
     returns (bool)
   {
-    return Schnorr.verify(public_key, keccak256(message), c, s);
+    return Schnorr.verify(publicKey, keccak256(message), c, s);
   }
 }

processor/ethereum/deployer/contracts/Deployer.sol

@@ -2,7 +2,7 @@
 pragma solidity ^0.8.26;
 
 /*
-  The expected deployment process of the Router is as follows:
+  The expected deployment process of Serai's Router is as follows:
 
   1) A transaction deploying Deployer is made. Then, a deterministic signature is
      created such that an account with an unknown private key is the creator of
@@ -32,35 +32,57 @@ pragma solidity ^0.8.26;
   with Serai verifying the published result. This would introduce a DoS risk in
   the council not publishing the correct key/not publishing any key.
 
-  This design does not work with designs expecting initialization (which may require re-deploying
+  This design does not work (well) with contracts expecting initialization due
-  the same code until the initialization successfully goes through, without being sniped).
+  to only allowing deploying init code once (which assumes contracts are
+  distinct via their constructors). Such designs are unused by Serai so that is
+  accepted.
 */
 
+/// @title Deployer of contracts for the Serai network
+/// @author Luke Parker <lukeparker@serai.exchange>
 contract Deployer {
+  /// @return The deployment for some (hashed) init code
   mapping(bytes32 => address) public deployments;
 
+  /// @notice Raised if the provided init code was already prior deployed
   error PriorDeployed();
+  /// @notice Raised if the deployment fails
   error DeploymentFailed();
 
-  function deploy(bytes memory init_code) external {
+  /// @notice Deploy the specified init code with `CREATE`
+  /// @dev This init code is expected to be unique and not prior deployed
+  /// @param initCode The init code to pass to `CREATE`
+  function deploy(bytes memory initCode) external {
     // Deploy the contract
-    address created_contract;
+    address createdContract;
+    // slither-disable-next-line assembly
     assembly {
-      created_contract := create(0, add(init_code, 0x20), mload(init_code))
+      createdContract := create(0, add(initCode, 0x20), mload(initCode))
     }
-    if (created_contract == address(0)) {
+    if (createdContract == address(0)) {
       revert DeploymentFailed();
     }
 
-    bytes32 init_code_hash = keccak256(init_code);
+    bytes32 initCodeHash = keccak256(initCode);
 
-    // Check this wasn't prior deployed
+    /*
-    // We check this *after* deploymeing (in violation of CEI) to handle re-entrancy related bugs
+      Check this wasn't prior deployed.
-    if (deployments[init_code_hash] != address(0)) {
+
+      This is a post-check, not a pre-check (in violation of the CEI pattern). If we used a
+      pre-check, a deployed contract could re-enter the Deployer to deploy the same contract
+      multiple times due to the inner call updating state and then the outer call overwriting it.
+      The post-check causes the outer call to error once the inner call updates state.
+
+      This does mean contract deployment may fail if deployment causes arbitrary execution which
+      maliciously nests deployment of the being-deployed contract. Such an inner call won't fail,
+      yet the outer call would. The usage of a re-entrancy guard would call the inner call to fail
+      while the outer call succeeds. This is considered so edge-case it isn't worth handling.
+    */
+    if (deployments[initCodeHash] != address(0)) {
       revert PriorDeployed();
     }
 
     // Write the deployment to storage
-    deployments[init_code_hash] = created_contract;
+    deployments[initCodeHash] = createdContract;
   }
 }

processor/ethereum/deployer/src/lib.rs

@@ -26,8 +26,8 @@ mod abi {
 
 /// The Deployer contract for the Serai Router contract.
 ///
-/// This Deployer has a deterministic address, letting it be immediately identified on any
+/// This Deployer has a deterministic address, letting it be immediately identified on any instance
-/// compatible chain. It then supports retrieving the Router contract's address (which isn't
+/// of the EVM. It then supports retrieving the deployed contracts addresses (which aren't
 /// deterministic) using a single call.
 #[derive(Clone, Debug)]
 pub struct Deployer(Arc<RootProvider<SimpleRequest>>);
@@ -66,6 +66,8 @@ impl Deployer {
   }
 
   /// Construct a new view of the Deployer.
+  ///
+  /// This will return None if the Deployer has yet to be deployed on-chain.
   pub async fn new(
     provider: Arc<RootProvider<SimpleRequest>>,
   ) -> Result<Option<Self>, RpcError<TransportErrorKind>> {

processor/ethereum/router/README.md

@@ -1 +1,5 @@
 # Ethereum Router
+
+The [Router contract](./contracts/Router.sol) is extensively documented to ensure clarity and
+understanding of the design decisions made. Please refer to it for understanding of why/what this
+is.

processor/ethereum/router/contracts/Router.sol

@@ -5,236 +5,493 @@ import "IERC20.sol";
 
 import "Schnorr.sol";
 
-// _ is used as a prefix for internal functions and smart-contract-scoped variables
+/*
-contract Router {
+  The Router directly performs low-level calls in order to fine-tune the gas settings. Since this
-  // Nonce is incremented for each command executed, preventing replays
+  contract is meant to relay an entire batch of transactions, the ability to exactly meter
-  uint256 private _nonce;
+  individual transactions is critical.
 
-  // The nonce which will be used for the smart contracts we deploy, enabling
+  We don't check the return values as we don't care if the calls succeeded. We solely care we made
-  // predicting their addresses
+  them. If someone configures an external contract in a way which borks, we epxlicitly define that
+  as their fault and out-of-scope to this contract.
+
+  If an actual invariant within Serai exists, an escape hatch exists to move to a new contract. Any
+  improperly handled actions can be re-signed and re-executed at that point in time.
+*/
+// slither-disable-start low-level-calls,unchecked-lowlevel
+
+/// @title Serai Router
+/// @author Luke Parker <lukeparker@serai.exchange>
+/// @notice Intakes coins for the Serai network and handles relaying batches of transfers out
+contract Router {
+  /**
+   * @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;
 
-  // The current public key, defined as per the Schnorr library
+  /// @dev A nonce incremented upon an action to prevent replays/out-of-order execution
+  uint256 private _nonce;
+
+  /**
+   * @dev The current public key for Serai's Ethereum validator set, in the form the Schnorr library
+   *   expects
+   */
   bytes32 private _seraiKey;
 
+  /// @dev The address escaped to
+  address private _escapedTo;
+
+  /// @title The type of destination
+  /// @dev A destination is either an address or a blob of code to deploy and call
   enum DestinationType {
     Address,
     Code
   }
 
-  struct AddressDestination {
+  /// @title A code destination
-    address destination;
+  /**
-  }
+   * @dev If transferring an ERC20 to this destination, it will be transferred to the address the
-
+   *   code will be deployed to. If transferring ETH, it will be transferred with the deployment of
+   *   the code. `code` is deployed with CREATE (calling its constructor). The entire deployment
+   *   (and associated sandboxing) must consume less than `gasLimit` units of gas or it will revert.
+   */
   struct CodeDestination {
-    uint32 gas_limit;
+    uint32 gasLimit;
     bytes code;
   }
 
+  /// @title An instruction to transfer coins out
+  /// @dev Specifies a destination and amount but not the coin as that's assumed to be contextual
   struct OutInstruction {
     DestinationType destinationType;
     bytes destination;
-    uint256 value;
+    uint256 amount;
   }
 
+  /// @title A signature
+  /// @dev Thin wrapper around `c, s` to simplify the API
   struct Signature {
     bytes32 c;
     bytes32 s;
   }
 
+  /// @notice Emitted when the key for Serai's Ethereum validators is updated
+  /// @param nonce The nonce consumed to update this key
+  /// @param key The key updated to
   event SeraiKeyUpdated(uint256 indexed nonce, bytes32 indexed key);
+
+  /// @notice Emitted when an InInstruction occurs
+  /// @param from The address which called `inInstruction` and caused this event to be emitted
+  /// @param coin The coin transferred in
+  /// @param amount The amount of the coin transferred in
+  /// @param instruction The Shorthand-encoded InInstruction for Serai to decode and handle
   event InInstruction(
     address indexed from, address indexed coin, uint256 amount, bytes instruction
   );
-  event Executed(uint256 indexed nonce, bytes32 indexed message_hash);
 
+  /// @notice Emitted when a batch of `OutInstruction`s occurs
+  /// @param nonce The nonce consumed to execute this batch of transactions
+  /// @param messageHash The hash of the message signed for the executed batch
+  event Executed(uint256 indexed nonce, bytes32 indexed messageHash);
+
+  /// @notice Emitted when `escapeHatch` is invoked
+  /// @param escapeTo The address to escape to
+  event EscapeHatch(address indexed escapeTo);
+
+  /// @notice Emitted when coins escape through the escape hatch
+  /// @param coin The coin which escaped
+  event Escaped(address indexed coin);
+
+  /// @notice The contract has had its escape hatch invoked and won't accept further actions
+  error EscapeHatchInvoked();
+  /// @notice The signature was invalid
   error InvalidSignature();
-  error InvalidAmount();
+  /// @notice The amount specified didn't match `msg.value`
-  error FailedTransfer();
+  error AmountMismatchesMsgValue();
+  /// @notice The call to an ERC20's `transferFrom` failed
+  error TransferFromFailed();
 
-  // Update the Serai key at the end of the current function.
+  /// @notice An invalid address to escape to was specified.
-  modifier _updateSeraiKeyAtEndOfFn(uint256 nonceUpdatedWith, bytes32 newSeraiKey) {
+  error InvalidEscapeAddress();
-    // Run the function itself.
+  /// @notice Escaping when escape hatch wasn't invoked.
+  error EscapeHatchNotInvoked();
+
+  /**
+   * @dev Updates the Serai key at the end of the current function. Executing at the end of the
+   *   current function allows verifying a signature with the current key. This does not update
+   *   `_nonce`
+   */
+  /// @param nonceUpdatedWith The nonce used to update the key
+  /// @param newSeraiKey The key updated to
+  modifier updateSeraiKeyAtEndOfFn(uint256 nonceUpdatedWith, bytes32 newSeraiKey) {
+    // Run the function itself
     _;
 
-    // Update the key.
+    // Update the key
     _seraiKey = newSeraiKey;
     emit SeraiKeyUpdated(nonceUpdatedWith, newSeraiKey);
   }
 
-  constructor(bytes32 initialSeraiKey) _updateSeraiKeyAtEndOfFn(0, initialSeraiKey) {
+  /// @notice The constructor for the relayer
-    // We consumed nonce 0 when setting the initial Serai key
+  /// @param initialSeraiKey The initial key for Serai's Ethereum validators
-    _nonce = 1;
+  constructor(bytes32 initialSeraiKey) updateSeraiKeyAtEndOfFn(0, initialSeraiKey) {
     // 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;
+
+    // We consumed nonce 0 when setting the initial Serai key
+    _nonce = 1;
+
+    // We haven't escaped to any address yet
+    _escapedTo = address(0);
   }
 
-  // updateSeraiKey validates the given Schnorr signature against the current public key, and if
+  /// @dev Verify a signature
-  // successful, updates the contract's public key to the one specified.
+  /// @param message The message to pass to the Schnorr contract
-  function updateSeraiKey(bytes32 newSeraiKey, Signature calldata signature)
+  /// @param signature The signature by the current key for this message
-    external
+  function verifySignature(bytes32 message, Signature calldata signature) private {
-    _updateSeraiKeyAtEndOfFn(_nonce, newSeraiKey)
+    // If the escape hatch was triggered, reject further signatures
-  {
+    if (_escapedTo != address(0)) {
-    // This DST needs a length prefix as well to prevent DSTs potentially being substrings of each
+      revert EscapeHatchInvoked();
-    // other, yet this fine for our very well-defined, limited use
+    }
-    bytes32 message =
+    // Verify the signature
-      keccak256(abi.encodePacked("updateSeraiKey", block.chainid, _nonce, newSeraiKey));
-    _nonce++;
-
     if (!Schnorr.verify(_seraiKey, message, signature.c, signature.s)) {
       revert InvalidSignature();
     }
+    // Increment the nonce
+    unchecked {
+      _nonce++;
+    }
   }
 
+  /// @notice Update the key representing Serai's Ethereum validators
+  /// @param newSeraiKey The key to update to
+  /// @param signature The signature by the current key authorizing this update
+  function updateSeraiKey(bytes32 newSeraiKey, Signature calldata signature)
+    external
+    updateSeraiKeyAtEndOfFn(_nonce, newSeraiKey)
+  {
+    /*
+      This DST needs a length prefix as well to prevent DSTs potentially being substrings of each
+      other, yet this is fine for our well-defined, extremely-limited use.
+
+      We don't encode the chain ID as Serai generates independent keys for each integration. If
+      Ethereum L2s are integrated, and they reuse the Ethereum validator set, we would use the
+      existing Serai key yet we'd apply an off-chain derivation scheme to bind it to specific
+      networks. This also lets Serai identify EVMs per however it wants, solving the edge case where
+      two instances of the EVM share a chain ID for whatever horrific reason.
+
+      This uses encodePacked as all items present here are of fixed length.
+    */
+    bytes32 message = keccak256(abi.encodePacked("updateSeraiKey", _nonce, newSeraiKey));
+    verifySignature(message, signature);
+  }
+
+  /// @notice Transfer coins into Serai with an instruction
+  /// @param coin The coin to transfer in (address(0) if Ether)
+  /// @param amount The amount to transfer in (msg.value if Ether)
+  /**
+   * @param instruction The Shorthand-encoded InInstruction for Serai to associate with this
+   *   transfer in
+   */
+  // Re-entrancy doesn't bork this function
+  // slither-disable-next-line reentrancy-events
   function inInstruction(address coin, uint256 amount, bytes memory instruction) external payable {
+    // Check the transfer
     if (coin == address(0)) {
-      if (amount != msg.value) revert InvalidAmount();
+      if (amount != msg.value) revert AmountMismatchesMsgValue();
     } else {
       (bool success, bytes memory res) = address(coin).call(
         abi.encodeWithSelector(IERC20.transferFrom.selector, msg.sender, address(this), amount)
       );
 
-      // Require there was nothing returned, which is done by some non-standard tokens, or that the
+      /*
-      // ERC20 contract did in fact return true
+        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 TransferFromFailed();
     }
 
     /*
-    Due to fee-on-transfer tokens, emitting the amount directly is frowned upon. The amount
+      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.
+      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
+      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
+      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
+      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).
+      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
+      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
+      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
+      would believe all transfers were done in full, despite each only being done in part (except
-    for the last one).
+      for the last one).
 
-    Given fee-on-transfer tokens aren't intended to be supported, the only token actively planned
+      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
+      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
+      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
+      classify this entire class of tokens as non-standard implementations which induce undefined
-    behavior.
+      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);
   }
 
-  /*
+  /// @dev Perform an ERC20 transfer out
-    We on purposely do not check if these calls succeed. A call either succeeded, and there's no
+  /// @param to The address to transfer the coins to
-    problem, or the call failed due to:
+  /// @param coin The coin to transfer
-      A) An insolvency
+  /// @param amount The amount of the coin to transfer
-      B) A malicious receiver
+  function erc20TransferOut(address to, address coin, uint256 amount) private {
-      C) A non-standard token
+    /*
-    A is an invariant, B should be dropped, C is something out of the control of this contract.
+      The ERC20s integrated are presumed to have a constant gas cost, meaning this can only be
-    It is again the Serai's network role to not add support for any non-standard tokens,
+      insufficient:
-  */
 
-  // Perform an ERC20 transfer out
+        A) An integrated ERC20 uses more gas than this limit (presumed not to be the case)
-  function _erc20TransferOut(address to, address coin, uint256 value) private {
+        B) An integrated ERC20 is upgraded (integrated ERC20s are presumed to not be upgradeable)
-    coin.call{ gas: 100_000 }(abi.encodeWithSelector(IERC20.transfer.selector, msg.sender, value));
+        C) This has a variable gas cost and the user set a hook on receive which caused this (in
-  }
+           which case, we accept dropping this)
+        D) The user was blacklisted (in which case, we again accept dropping this)
+        E) Other extreme edge cases, for which such tokens are assumed to not be integrated
+        F) Ethereum opcodes are repriced in a sufficiently breaking fashion
 
-  // Perform an ETH/ERC20 transfer out
+      This should be in such excess of the gas requirements of integrated tokens we'll survive
-  function _transferOut(address to, address coin, uint256 value) private {
+      repricing, so long as the repricing doesn't revolutionize EVM gas costs as we know it. In such
-    if (coin == address(0)) {
+      a case, Serai would have to migrate to a new smart contract using `escapeHatch`.
-      // Enough gas to service the transfer and a minimal amount of logic
+    */
-      to.call{ value: value, gas: 5_000 }("");
+    uint256 _gas = 100_000;
-    } else {
+
-      _erc20TransferOut(to, coin, value);
+    bytes memory _calldata = abi.encodeWithSelector(IERC20.transfer.selector, to, amount);
+    bool _success;
+    // slither-disable-next-line assembly
+    assembly {
+      /*
+        `coin` is trusted so we can accept the risk of a return bomb here, yet we won't check the
+        return value anyways so there's no need to spend the gas decoding it. We assume failures
+        are the fault of the recipient, not us, the sender. We don't want to have such errors block
+        the queue of transfers to make.
+
+        If there ever was some invariant broken, off-chain actions is presumed to occur to move to a
+        new smart contract with whatever necessary changes made/response occurring.
+      */
+      _success :=
+        call(
+          _gas,
+          coin,
+          // Ether value
+          0,
+          // calldata
+          add(_calldata, 0x20),
+          mload(_calldata),
+          // return data
+          0,
+          0
+        )
     }
   }
 
-  /*
+  /// @dev Perform an ETH/ERC20 transfer out
-  Serai supports arbitrary calls out via deploying smart contracts (with user-specified code),
+  /// @param to The address to transfer the coins to
-  letting them execute whatever calls they're coded for. Since we can't meter CREATE, we call
+  /// @param coin The coin to transfer (address(0) if Ether)
-  CREATE from this function which we call not internally, but with CALL (which we can meter).
+  /// @param amount The amount of the coin to transfer
-  */
+  function transferOut(address to, address coin, uint256 amount) private {
-  function arbitaryCallOut(bytes memory code) external payable {
+    if (coin == address(0)) {
+      // Enough gas to service the transfer and a minimal amount of logic
+      uint256 _gas = 5_000;
+      // This uses assembly to prevent return bombs
+      bool _success;
+      // slither-disable-next-line assembly
+      assembly {
+        _success :=
+          call(
+            _gas,
+            to,
+            amount,
+            // calldata
+            0,
+            0,
+            // return data
+            0,
+            0
+          )
+      }
+    } else {
+      erc20TransferOut(to, coin, amount);
+    }
+  }
+
+  /// @notice Execute some arbitrary code within a secure sandbox
+  /**
+   * @dev This performs sandboxing by deploying this code with `CREATE`. This is an external
+   *   function as we can't meter `CREATE`/internal functions. We work around this by calling this
+   *   function with `CALL` (which we can meter). This does forward `msg.value` to the newly
+   *  deployed contract.
+   */
+  /// @param code The code to execute
+  function executeArbitraryCode(bytes memory code) external payable {
     // Because we're creating a contract, increment our nonce
     _smartContractNonce += 1;
 
-    uint256 msg_value = msg.value;
+    uint256 msgValue = msg.value;
     address contractAddress;
+    // We need to use assembly here because Solidity doesn't expose CREATE
+    // slither-disable-next-line assembly
     assembly {
-      contractAddress := create(msg_value, add(code, 0x20), mload(code))
+      contractAddress := create(msgValue, add(code, 0x20), mload(code))
     }
   }
 
-  // Execute a list of transactions if they were signed by the current key with the current nonce
+  /// @notice Execute a batch of `OutInstruction`s
+  /**
+   * @dev All `OutInstruction`s in a batch are only for a single coin to simplify handling of the
+   *  fee
+   */
+  /// @param coin The coin all of these `OutInstruction`s are for
+  /// @param fee The fee to pay (in coin) to the caller for their relaying of this batch
+  /// @param outs The `OutInstruction`s to act on
+  /// @param signature The signature by the current key for Serai's Ethereum validators
+  // Each individual call is explicitly metered to ensure there isn't a DoS here
+  // slither-disable-next-line calls-loop
   function execute(
     address coin,
     uint256 fee,
-    OutInstruction[] calldata transactions,
+    OutInstruction[] calldata outs,
     Signature calldata signature
   ) external {
     // Verify the signature
-    // We hash the message here as we need the message's hash for the Executed event
+    // This uses `encode`, not `encodePacked`, as `outs` is of variable length
-    // Since we're already going to hash it, hashing it prior to verifying the signature reduces the
+    // TODO: Use a custom encode in verifySignature here with assembly (benchmarking before/after)
-    // amount of words hashed by its challenge function (reducing our gas costs)
+    bytes32 message = keccak256(abi.encode("execute", _nonce, coin, fee, outs));
-    bytes32 message =
+    verifySignature(message, signature);
-      keccak256(abi.encode("execute", block.chainid, _nonce, coin, fee, transactions));
-    if (!Schnorr.verify(_seraiKey, message, signature.c, signature.s)) {
-      revert InvalidSignature();
-    }
 
-    // Since the signature was verified, perform execution
+    // _nonce: Also include a bit mask here
     emit Executed(_nonce, 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++) {
+    /*
+      Since we don't have a re-entrancy guard, it is possible for instructions from later batches to
+      be executed before these instructions. This is deemed fine. We only require later batches be
+      relayed after earlier batches in order to form backpressure. This means if a batch has a fee
+      which isn't worth relaying the batch for, so long as later batches are sufficiently
+      worthwhile, every batch will be relayed.
+    */
+
+    // slither-disable-next-line reentrancy-events
+    for (uint256 i = 0; i < outs.length; i++) {
       // If the destination is an address, we perform a direct transfer
-      if (transactions[i].destinationType == DestinationType.Address) {
+      if (outs[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
+          This may cause a revert if the destination isn't actually a valid address. Serai is
-        (AddressDestination memory destination) =
+          trusted to not pass a malformed destination, yet if it ever did, it could simply re-sign a
-          abi.decode(transactions[i].destination, (AddressDestination));
+          corrected batch using this nonce.
-        _transferOut(destination.destination, coin, transactions[i].value);
+        */
+        address destination = abi.decode(outs[i].destination, (address));
+        transferOut(destination, coin, outs[i].amount);
       } else {
-        // Prepare for the transfer
+        // Prepare the transfer
-        uint256 eth_value = 0;
+        uint256 ethValue = 0;
         if (coin == address(0)) {
-          // If it's ETH, we transfer the value with the call
+          // If it's ETH, we transfer the amount with the call
-          eth_value = transactions[i].value;
+          ethValue = outs[i].amount;
         } else {
-          // If it's an ERC20, we calculate the hash of the will-be contract and transfer to it
+          /*
-          // before deployment. This avoids needing to deploy, then call again, offering a few
+            If it's an ERC20, we calculate the address of the will-be contract and transfer to it
-          // optimizations
+            before deployment. This avoids needing to deploy the contract, then call transfer, then
-          address nextAddress =
+            call the contract again
-            address(uint160(uint256(keccak256(abi.encode(address(this), _smartContractNonce)))));
+          */
-          _erc20TransferOut(nextAddress, coin, transactions[i].value);
+          address nextAddress = address(
+            uint160(uint256(keccak256(abi.encodePacked(address(this), _smartContractNonce))))
+          );
+          erc20TransferOut(nextAddress, coin, outs[i].amount);
         }
 
-        // Perform the deployment with the defined gas budget
+        (CodeDestination memory destination) = abi.decode(outs[i].destination, (CodeDestination));
-        (CodeDestination memory destination) =
+
-          abi.decode(transactions[i].destination, (CodeDestination));
+        /*
-        address(this).call{ gas: destination.gas_limit, value: eth_value }(
+          Perform the deployment with the defined gas budget.
-          abi.encodeWithSelector(Router.arbitaryCallOut.selector, destination.code)
+
+          We don't care if the following call fails as we don't want to block/retry if it does.
+          Failures are considered the recipient's fault. We explicitly do not want the surface
+          area/inefficiency of caching these for later attempted retires.
+
+          We don't have to worry about a return bomb here as this is our own function which doesn't
+          return any data.
+        */
+        address(this).call{ gas: destination.gasLimit, value: ethValue }(
+          abi.encodeWithSelector(Router.executeArbitraryCode.selector, destination.code)
         );
       }
     }
 
-    // Transfer to the caller the fee
+    // Transfer the fee to the relayer
-    _transferOut(msg.sender, coin, fee);
+    transferOut(msg.sender, coin, fee);
   }
 
+  /// @notice Escapes to a new smart contract
+  /// @dev This should be used upon an invariant being reached or new functionality being needed
+  /// @param escapeTo The address to escape to
+  /// @param signature The signature by the current key for Serai's Ethereum validators
+  function escapeHatch(address escapeTo, Signature calldata signature) external {
+    if (escapeTo == address(0)) {
+      revert InvalidEscapeAddress();
+    }
+    /*
+      We want to define the escape hatch so coins here now, and latently received, can be forwarded.
+      If the last Serai key set could update the escape hatch, they could siphon off latently
+      received coins without penalty (if they update the escape hatch after unstaking).
+    */
+    if (_escapedTo != address(0)) {
+      revert EscapeHatchInvoked();
+    }
+
+    // Verify the signature
+    bytes32 message = keccak256(abi.encodePacked("escapeHatch", _nonce, escapeTo));
+    verifySignature(message, signature);
+
+    _escapedTo = escapeTo;
+    emit EscapeHatch(escapeTo);
+  }
+
+  /// @notice Escape coins after the escape hatch has been invoked
+  /// @param coin The coin to escape
+  function escape(address coin) external {
+    if (_escapedTo == address(0)) {
+      revert EscapeHatchNotInvoked();
+    }
+
+    emit Escaped(coin);
+
+    // Fetch the amount to escape
+    uint256 amount = address(this).balance;
+    if (coin != address(0)) {
+      amount = IERC20(coin).balanceOf(address(this));
+    }
+
+    // Perform the transfer
+    transferOut(_escapedTo, coin, amount);
+  }
+
+  /// @notice Fetch the next nonce to use by an action published to this contract
+  /// return The next nonce to use by an action published to this contract
   function nonce() external view returns (uint256) {
     return _nonce;
   }
 
-  function smartContractNonce() external view returns (uint256) {
+  /// @notice Fetch the current key for Serai's Ethereum validator set
-    return _smartContractNonce;
+  /// @return The current key for Serai's Ethereum validator set
-  }
-
   function seraiKey() external view returns (bytes32) {
     return _seraiKey;
   }
+
+  /// @notice Fetch the address escaped to
+  /// @return The address which was escaped to (address(0) if the escape hatch hasn't been invoked)
+  function escapedTo() external view returns (address) {
+    return _escapedTo;
+  }
 }
+
+// slither-disable-end low-level-calls,unchecked-lowlevel

processor/ethereum/router/src/lib.rs

@@ -297,10 +297,9 @@ impl Router {
   }
 
   /// Get the message to be signed in order to update the key for Serai.
-  pub fn update_serai_key_message(chain_id: U256, nonce: u64, key: &PublicKey) -> Vec<u8> {
+  pub fn update_serai_key_message(nonce: u64, key: &PublicKey) -> Vec<u8> {
     (
       "updateSeraiKey",
-      chain_id,
       U256::try_from(nonce).expect("couldn't convert u64 to u256"),
       key.eth_repr(),
     )
@@ -322,13 +321,12 @@ 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,
     coin: Coin,
     fee: U256,
     outs: OutInstructions,
   ) -> Vec<u8> {
-    ("execute", chain_id, U256::try_from(nonce).unwrap(), coin.address(), fee, outs.0).abi_encode()
+    ("execute", U256::try_from(nonce).unwrap(), coin.address(), fee, outs.0).abi_encode()
   }
 
   /// Construct a transaction to execute a batch of `OutInstruction`s.