Simplify and test deterministically_sign

processor/ethereum/TODO/old_processor.rs

@@ -26,7 +26,7 @@ TODO
       };
       tx.gas_limit = 1_000_000u64.into();
       tx.gas_price = 1_000_000_000u64.into();
-      let tx = ethereum_serai::crypto::deterministically_sign(&tx);
+      let tx = ethereum_serai::crypto::deterministically_sign(tx);
 
       if self.provider.get_transaction_by_hash(*tx.hash()).await.unwrap().is_none() {
         self

processor/ethereum/TODO/tests/mod.rs

@@ -109,7 +109,7 @@ pub async fn deploy_contract(
     input: bin,
   };
 
-  let deployment_tx = deterministically_sign(&deployment_tx);
+  let deployment_tx = deterministically_sign(deployment_tx);
 
   // Fund the deployer address
   fund_account(

processor/ethereum/deployer/src/lib.rs

@@ -43,10 +43,13 @@ impl Deployer {
     let bytecode =
       Bytes::from_hex(BYTECODE).expect("compiled-in Deployer bytecode wasn't valid hex");
 
+    // Legacy transactions are used to ensure the widest possible degree of support across EVMs
     let tx = TxLegacy {
       chain_id: None,
       nonce: 0,
-      // 100 gwei
+      // This uses a fixed gas price as necessary to achieve a deterministic address
+      // The gas price is fixed to 100 gwei, which should be incredibly generous, in order to make
+      // this getting stuck unlikely. While expensive, this only has to occur once
       gas_price: 100_000_000_000u128,
       // TODO: Use a more accurate gas limit
       gas_limit: 1_000_000u64,
@@ -55,7 +58,7 @@ impl Deployer {
       input: bytecode,
     };
 
-    ethereum_primitives::deterministically_sign(&tx)
+    ethereum_primitives::deterministically_sign(tx)
   }
 
   /// Obtain the deterministic address for this contract.

processor/ethereum/primitives/src/lib.rs

@@ -15,34 +15,66 @@ pub fn keccak256(data: impl AsRef<[u8]>) -> [u8; 32] {
 
 /// Deterministically sign a transaction.
 ///
-/// This signs a transaction via setting `r = 1, s = 1`, and incrementing `r` until a signer is
-/// recoverable from the signature for this transaction. The purpose of this is to be able to send
-/// a transaction from a known account which no one knows the private key for.
+/// This signs a transaction via setting a signature of `r = 1, s = 1`. The purpose of this is to
+/// be able to send a transaction from an account which no one knows the private key for and no
+/// other messages may be signed for from.
 ///
 /// This function panics if passed a transaction with a non-None chain ID. This is because the
 /// signer for this transaction is only singular across any/all EVM instances if it isn't binding
 /// to an instance.
-pub fn deterministically_sign(tx: &TxLegacy) -> Signed<TxLegacy> {
+pub fn deterministically_sign(tx: TxLegacy) -> Signed<TxLegacy> {
   assert!(
     tx.chain_id.is_none(),
     "chain ID was Some when deterministically signing a TX (causing a non-singular signer)"
   );
 
-  let mut r = Scalar::ONE;
+  /*
+    ECDSA signatures are:
+    - x = private key
+    - k = rand()
+    - R = k * G
+    - r = R.x()
+    - s = (H(m) + (r * x)) * k.invert()
+
+    Key recovery is performed via:
+    - a = s * R = (H(m) + (r * x)) * G
+    - b = a - (H(m) * G) = (r * x) * G
+    - X = b / r = x * G
+    - X = ((s * R) - (H(m) * G)) * r.invert()
+
+    This requires `r` be non-zero and `R` be recoverable from `r` and the parity byte. For
+    `r = 1, s = 1`, this sets `X` to `R - (H(m) * G)`. Since there is an `R` recoverable for
+    `r = 1`, since the `R` is a point with an unknown discrete logarithm w.r.t. the generator, and
+    since the resulting key is dependent on the message signed for, this will always work to
+    the specification.
+  */
+
+  let r = Scalar::ONE;
   let s = Scalar::ONE;
-  loop {
-    // Create the signature
-    let r_bytes: [u8; 32] = r.to_repr().into();
-    let s_bytes: [u8; 32] = s.to_repr().into();
-    let signature =
-      PrimitiveSignature::from_scalars_and_parity(r_bytes.into(), s_bytes.into(), false);
+  let r_bytes: [u8; 32] = r.to_repr().into();
+  let s_bytes: [u8; 32] = s.to_repr().into();
+  let signature =
+    PrimitiveSignature::from_scalars_and_parity(r_bytes.into(), s_bytes.into(), false);
 
-    // Check if this is a valid signature
-    let tx = tx.clone().into_signed(signature);
-    if tx.recover_signer().is_ok() {
-      return tx;
-    }
-
-    r += Scalar::ONE;
-  }
+  let res = tx.into_signed(signature);
+  debug_assert!(res.recover_signer().is_ok());
+  res
+}
+
+#[test]
+fn test_deterministically_sign() {
+  let tx = TxLegacy { chain_id: None, ..Default::default() };
+  let signed = deterministically_sign(tx.clone());
+
+  assert!(signed.recover_signer().is_ok());
+  let one = alloy_core::primitives::U256::from(1u64);
+  assert_eq!(signed.signature().r(), one);
+  assert_eq!(signed.signature().s(), one);
+
+  let mut other_tx = tx.clone();
+  other_tx.nonce += 1;
+  // Signing a distinct message should yield a distinct signer
+  assert!(
+    signed.recover_signer().unwrap() != deterministically_sign(other_tx).recover_signer().unwrap()
+  );
 }

processor/ethereum/router/src/tests/mod.rs

@@ -84,7 +84,7 @@ async fn setup_test(
   // Set a gas price (100 gwei)
   tx.gas_price = 100_000_000_000;
   // Sign it
-  let tx = ethereum_primitives::deterministically_sign(&tx);
+  let tx = ethereum_primitives::deterministically_sign(tx);
   // Publish it
   let receipt = ethereum_test_primitives::publish_tx(&provider, tx).await;
   assert!(receipt.status());
@@ -123,7 +123,7 @@ async fn confirm_next_serai_key(
 
   let mut tx = router.confirm_next_serai_key(&sig);
   tx.gas_price = 100_000_000_000;
-  let tx = ethereum_primitives::deterministically_sign(&tx);
+  let tx = ethereum_primitives::deterministically_sign(tx);
   let receipt = ethereum_test_primitives::publish_tx(provider, tx).await;
   assert!(receipt.status());
   assert_eq!(
@@ -164,7 +164,7 @@ async fn test_update_serai_key() {
 
   let mut tx = router.update_serai_key(&update_to, &sig);
   tx.gas_price = 100_000_000_000;
-  let tx = ethereum_primitives::deterministically_sign(&tx);
+  let tx = ethereum_primitives::deterministically_sign(tx);
   let receipt = ethereum_test_primitives::publish_tx(&provider, tx).await;
   assert!(receipt.status());
   assert_eq!(u128::from(Router::UPDATE_SERAI_KEY_GAS), ((receipt.gas_used + 1000) / 1000) * 1000);
@@ -199,7 +199,7 @@ async fn test_eth_in_instruction() {
     .abi_encode()
     .into(),
   };
-  let tx = ethereum_primitives::deterministically_sign(&tx);
+  let tx = ethereum_primitives::deterministically_sign(tx);
   let signer = tx.recover_signer().unwrap();
 
   let receipt = ethereum_test_primitives::publish_tx(&provider, tx).await;
@@ -250,7 +250,7 @@ async fn publish_outs(
 
   let mut tx = router.execute(coin, fee, outs, &sig);
   tx.gas_price = 100_000_000_000;
-  let tx = ethereum_primitives::deterministically_sign(&tx);
+  let tx = ethereum_primitives::deterministically_sign(tx);
   ethereum_test_primitives::publish_tx(provider, tx).await
 }
 
@@ -307,7 +307,7 @@ async fn escape_hatch(
 
   let mut tx = router.escape_hatch(escape_to, &sig);
   tx.gas_price = 100_000_000_000;
-  let tx = ethereum_primitives::deterministically_sign(&tx);
+  let tx = ethereum_primitives::deterministically_sign(tx);
   let receipt = ethereum_test_primitives::publish_tx(provider, tx).await;
   assert!(receipt.status());
   assert_eq!(u128::from(Router::ESCAPE_HATCH_GAS), ((receipt.gas_used + 1000) / 1000) * 1000);
@@ -321,7 +321,7 @@ async fn escape(
 ) -> TransactionReceipt {
   let mut tx = router.escape(coin.address());
   tx.gas_price = 100_000_000_000;
-  let tx = ethereum_primitives::deterministically_sign(&tx);
+  let tx = ethereum_primitives::deterministically_sign(tx);
   let receipt = ethereum_test_primitives::publish_tx(provider, tx).await;
   assert!(receipt.status());
   receipt

processor/ethereum/test-primitives/src/lib.rs

@@ -76,7 +76,7 @@ pub async fn deploy_contract(
     input: bin.into(),
   };
 
-  let deployment_tx = deterministically_sign(&deployment_tx);
+  let deployment_tx = deterministically_sign(deployment_tx);
 
   let receipt = publish_tx(provider, deployment_tx).await;
   assert!(receipt.status());