fix: make chain_id mandatory (#286)

* chore: update deps (#352)

* chore(deps): bump elliptic-curve from 0.10.4 to 0.10.5

Bumps [elliptic-curve](https://github.com/RustCrypto/traits) from 0.10.4 to 0.10.5.
- [Release notes](https://github.com/RustCrypto/traits/releases)
- [Commits](RustCrypto/traits@elliptic-curve-v0.10.4...elliptic-curve-v0.10.5)

---
updated-dependencies:
- dependency-name: elliptic-curve
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* chore: bump deps

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(core): make chain id mandatory

* fix(signers): make chain id mandatory

* test: make chain id mandatory

* test: add missing chain id

* fix: add missing chain id

* chore(wallet): set chain_id by default to 1

* ci: run CI on master

* fix(yubi): add missing chain id

* chore: skip ganache test with celo features

* ci: run only on push to master

* fix: add missing chain id

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

.github/workflows/ci.yml

@@ -1,4 +1,8 @@
-on: pull_request
+on:
+  push:
+    branches: 
+      - master
+  pull_request:
 
 name: Tests
 
@@ -10,7 +14,7 @@ env:
 
 jobs:
   tests:
-    name: Check
+    name: ethereum tests
     runs-on: ubuntu-latest
     steps:
       - name: Checkout sources
@@ -51,7 +55,7 @@ jobs:
             cargo test
 
   feature-tests:
-    name: Check
+    name: celo tests
     runs-on: ubuntu-latest
     steps:
       - name: Checkout sources
@@ -95,7 +99,7 @@ jobs:
             cargo test --all-features
 
   lint:
-    name: Check
+    name: lints
     runs-on: ubuntu-latest
     steps:
       - name: Checkout sources

ethers-contract/tests/common/mod.rs

@@ -8,7 +8,7 @@ use ethers_contract::{Contract, ContractFactory, EthEvent};
 use ethers_core::utils::{GanacheInstance, Solc};
 use ethers_middleware::signer::SignerMiddleware;
 use ethers_providers::{Http, Middleware, Provider};
-use ethers_signers::LocalWallet;
+use ethers_signers::{LocalWallet, Signer};
 use std::{convert::TryFrom, sync::Arc, time::Duration};
 
 // Note: The `EthEvent` derive macro implements the necessary conversion between `Tokens` and
@@ -40,6 +40,7 @@ pub fn connect(ganache: &GanacheInstance, idx: usize) -> Arc<HttpWallet> {
         .unwrap()
         .interval(Duration::from_millis(10u64));
     let wallet: LocalWallet = ganache.keys()[idx].clone().into();
+    let wallet = wallet.with_chain_id(1u64);
     Arc::new(SignerMiddleware::new(provider, wallet))
 }
 

ethers-contract/tests/contract.rs

@@ -97,12 +97,11 @@ mod eth_tests {
         let contract = deploy(client.clone(), abi, bytecode).await;
 
         // make a call with `client`
-        let _tx_hash = *contract
+        let func = contract
             .method::<_, H256>("setValue", "hi".to_owned())
-            .unwrap()
-            .send()
-            .await
             .unwrap();
+        let tx = func.send().await.unwrap();
+        let _receipt = tx.await.unwrap();
 
         // and we can fetch the events
         let logs: Vec<ValueChanged> = contract
@@ -504,8 +503,8 @@ mod celo_tests {
     use super::*;
     use ethers::{
         middleware::signer::SignerMiddleware,
-        providers::{Http, Provider},
-        signers::LocalWallet,
+        providers::{Http, Middleware, Provider},
+        signers::{LocalWallet, Signer},
         types::BlockNumber,
     };
     use std::{convert::TryFrom, sync::Arc, time::Duration};
@@ -518,12 +517,13 @@ mod celo_tests {
         let provider = Provider::<Http>::try_from("https://alfajores-forno.celo-testnet.org")
             .unwrap()
             .interval(Duration::from_millis(6000));
+        let chain_id = provider.get_chainid().await.unwrap().as_u64();
 
         // Funded with https://celo.org/developers/faucet
         let wallet = "d652abb81e8c686edba621a895531b1f291289b63b5ef09a94f686a5ecdd5db1"
             .parse::<LocalWallet>()
             .unwrap()
-            .set_chain_id(44787u64);
+            .with_chain_id(chain_id);
 
         let client = SignerMiddleware::new(provider, wallet);
         let client = Arc::new(client);

ethers-core/Cargo.toml

@@ -45,6 +45,7 @@ serde_json = { version = "1.0.64", default-features = false }
 bincode = { version = "1.3.3", default-features = false }
 once_cell = { version = "1.8.0" }
 hex-literal = "0.3.3"
+futures-util = { version = "0.3.16", default-features = false }
 
 [features]
 celo = [] # celo support extends the transaction format with extra fields

ethers-core/src/types/transaction.rs

@@ -7,15 +7,14 @@ use crate::{
 use rlp::RlpStream;
 use serde::{Deserialize, Serialize};
 
+const BASE_NUM_TX_FIELDS: usize = 9;
+
 // Number of tx fields before signing
 #[cfg(not(feature = "celo"))]
-const UNSIGNED_TX_FIELDS: usize = 6;
+const NUM_TX_FIELDS: usize = BASE_NUM_TX_FIELDS;
 // Celo has 3 additional fields
 #[cfg(feature = "celo")]
-const UNSIGNED_TX_FIELDS: usize = 9;
-
-// Unsigned fields + signature [r s v]
-const SIGNED_TX_FIELDS: usize = UNSIGNED_TX_FIELDS + 3;
+const NUM_TX_FIELDS: usize = BASE_NUM_TX_FIELDS + 3;
 
 /// Parameters for sending a transaction
 #[derive(Clone, Default, Serialize, Deserialize, PartialEq, Eq, Debug)]
@@ -130,52 +129,34 @@ impl TransactionRequest {
     }
 
     /// Hashes the transaction's data with the provided chain id
-    pub fn sighash<T: Into<U64>>(&self, chain_id: Option<T>) -> H256 {
+    pub fn sighash<T: Into<U64>>(&self, chain_id: T) -> H256 {
         keccak256(self.rlp(chain_id).as_ref()).into()
     }
 
     /// Gets the unsigned transaction's RLP encoding
-    pub fn rlp<T: Into<U64>>(&self, chain_id: Option<T>) -> Bytes {
+    pub fn rlp<T: Into<U64>>(&self, chain_id: T) -> Bytes {
         let mut rlp = RlpStream::new();
-        // "If [..] CHAIN_ID is available, then when  computing the hash of a
-        // transaction for the purposes of signing, instead of hashing only
-        // six rlp encoded elements (nonce, gasprice, startgas, to, value, data),
-        // you SHOULD hash nine rlp encoded elements
-        // (nonce, gasprice, startgas, to, value, data, chainid, 0, 0)"
-        // https://github.com/ethereum/EIPs/blob/master/EIPS/eip-155.md#specification
-        let num_els = if chain_id.is_some() {
-            UNSIGNED_TX_FIELDS + 3
-        } else {
-            UNSIGNED_TX_FIELDS
-        };
-
-        rlp.begin_list(num_els);
+        rlp.begin_list(NUM_TX_FIELDS);
         self.rlp_base(&mut rlp);
 
         // Only hash the 3 extra fields when preparing the
         // data to sign if chain_id is present
-        if let Some(chain_id) = chain_id {
-            rlp.append(&chain_id.into());
-            rlp.append(&0u8);
-            rlp.append(&0u8);
-        }
-
+        rlp.append(&chain_id.into());
+        rlp.append(&0u8);
+        rlp.append(&0u8);
         rlp.out().freeze().into()
     }
 
     /// Produces the RLP encoding of the transaction with the provided signature
     pub fn rlp_signed(&self, signature: &Signature) -> Bytes {
         let mut rlp = RlpStream::new();
-
-        // construct the RLP body
-        rlp.begin_list(SIGNED_TX_FIELDS);
+        rlp.begin_list(NUM_TX_FIELDS);
         self.rlp_base(&mut rlp);
 
         // append the signature
         rlp.append(&signature.v);
         rlp.append(&signature.r);
         rlp.append(&signature.s);
-
         rlp.out().freeze().into()
     }
 
@@ -326,7 +307,7 @@ impl Transaction {
 
     pub fn rlp(&self) -> Bytes {
         let mut rlp = RlpStream::new();
-        rlp.begin_list(SIGNED_TX_FIELDS);
+        rlp.begin_list(NUM_TX_FIELDS);
         rlp.append(&self.nonce);
         rlp.append(&self.gas_price);
         rlp.append(&self.gas);

ethers-middleware/Cargo.toml

@@ -36,7 +36,6 @@ tokio = { version = "1.5" }
 
 [dev-dependencies]
 ethers = { version = "0.4.0", path = "../ethers" }
-futures-executor = { version = "0.3.14", features = ["thread-pool"] }
 hex = { version = "0.4.3", default-features = false, features = ["std"] }
 rand = { version = "0.8.4", default-features = false }
 tokio = { version = "1.5", default-features = false, features = ["rt", "macros", "time"] }

ethers-middleware/src/lib.rs

@@ -21,7 +21,7 @@
 //! ```no_run
 //! use ethers::{
 //!     providers::{Provider, Http},
-//!     signers::LocalWallet,
+//!     signers::{LocalWallet, Signer},
 //!     middleware::{
 //!         gas_escalator::{GasEscalatorMiddleware, GeometricGasPrice, Frequency},
 //!         gas_oracle::{GasOracleMiddleware, GasNow, GasCategory},

ethers-middleware/src/signer.rs

@@ -343,7 +343,7 @@ mod tests {
         let key = "4c0883a69102937d6231471b5dbb6204fe5129617082792ae468d01a3f362318"
             .parse::<LocalWallet>()
             .unwrap()
-            .set_chain_id(chain_id);
+            .with_chain_id(chain_id);
         let client = SignerMiddleware::new(provider, key);
 
         let tx = client.sign_transaction(tx).await.unwrap();
@@ -365,7 +365,7 @@ mod tests {
 
         // new SignerMiddleware
         let provider = Provider::try_from("http://localhost:8545").unwrap();
-        let key = LocalWallet::new(&mut rand::thread_rng());
+        let key = LocalWallet::new(&mut rand::thread_rng()).with_chain_id(1u32);
         let client = SignerMiddleware::new(provider, key);
 
         // an address that is not the signer address

ethers-middleware/tests/gas_escalator.rs

@@ -4,7 +4,7 @@ use ethers_middleware::{
     signer::SignerMiddleware,
 };
 use ethers_providers::{Middleware, Provider, Ws};
-use ethers_signers::LocalWallet;
+use ethers_signers::{LocalWallet, Signer};
 use std::time::Duration;
 
 #[tokio::test]

ethers-middleware/tests/nonce_manager.rs

@@ -4,18 +4,20 @@ async fn nonce_manager() {
     use ethers_core::types::*;
     use ethers_middleware::{nonce_manager::NonceManagerMiddleware, signer::SignerMiddleware};
     use ethers_providers::{Http, Middleware, Provider};
-    use ethers_signers::LocalWallet;
+    use ethers_signers::{LocalWallet, Signer};
     use std::convert::TryFrom;
     use std::time::Duration;
 
     let provider =
         Provider::<Http>::try_from("https://rinkeby.infura.io/v3/fd8b88b56aa84f6da87b60f5441d6778")
             .unwrap()
             .interval(Duration::from_millis(2000u64));
+    let chain_id = provider.get_chainid().await.unwrap().as_u64();
 
     let wallet = "59c37cb6b16fa2de30675f034c8008f890f4b2696c729d6267946d29736d73e4"
         .parse::<LocalWallet>()
-        .unwrap();
+        .unwrap()
+        .with_chain_id(chain_id);
     let address = wallet.address();
 
     let provider = SignerMiddleware::new(provider, wallet);

ethers-middleware/tests/signer.rs

@@ -2,7 +2,7 @@ use ethers_providers::{Http, Middleware, Provider};
 
 use ethers_core::types::TransactionRequest;
 use ethers_middleware::signer::SignerMiddleware;
-use ethers_signers::LocalWallet;
+use ethers_signers::{LocalWallet, Signer};
 use std::{convert::TryFrom, time::Duration};
 
 #[tokio::test]
@@ -20,6 +20,8 @@ async fn send_eth() {
     let provider = Provider::<Http>::try_from(ganache.endpoint())
         .unwrap()
         .interval(Duration::from_millis(10u64));
+    let chain_id = provider.get_chainid().await.unwrap().as_u64();
+    let wallet = wallet.with_chain_id(chain_id);
     let provider = SignerMiddleware::new(provider, wallet);
 
     // craft the transaction
@@ -48,13 +50,14 @@ async fn test_send_transaction() {
     let provider = Provider::<Http>::try_from("https://alfajores-forno.celo-testnet.org")
         .unwrap()
         .interval(Duration::from_millis(3000u64));
+    let chain_id = provider.get_chainid().await.unwrap().as_u64();
 
     // Funded with https://celo.org/developers/faucet
     // Please do not drain this account :)
     let wallet = "d652abb81e8c686edba621a895531b1f291289b63b5ef09a94f686a5ecdd5db1"
         .parse::<LocalWallet>()
         .unwrap()
-        .set_chain_id(44787u64);
+        .with_chain_id(chain_id);
     let client = SignerMiddleware::new(provider, wallet);
 
     let balance_before = client.get_balance(client.address(), None).await.unwrap();
@@ -71,6 +74,7 @@ async fn test_send_transaction() {
 }
 
 #[tokio::test]
+#[cfg(not(feature = "celo"))]
 async fn send_transaction_handles_tx_from_field() {
     use ethers_core::utils::Ganache;
 

ethers-middleware/tests/stack.rs

@@ -8,7 +8,7 @@ mod tests {
         signer::SignerMiddleware,
     };
     use ethers_providers::{Http, Middleware, Provider};
-    use ethers_signers::LocalWallet;
+    use ethers_signers::{LocalWallet, Signer};
     use std::convert::TryFrom;
 
     #[tokio::test]
@@ -58,6 +58,8 @@ mod tests {
 
         // the base provider
         let provider = Arc::new(Provider::<Http>::try_from(ganache.endpoint()).unwrap());
+        let chain_id = provider.get_chainid().await.unwrap().as_u64();
+        let signer = signer.with_chain_id(chain_id);
 
         // the Gas Price escalator middleware is the first middleware above the provider,
         // so that it receives the transaction last, after all the other middleware

ethers-middleware/tests/transformer.rs

@@ -8,7 +8,7 @@ use ethers_middleware::{
     SignerMiddleware,
 };
 use ethers_providers::{Http, Middleware, Provider};
-use ethers_signers::LocalWallet;
+use ethers_signers::{LocalWallet, Signer};
 use rand::Rng;
 use std::{convert::TryFrom, sync::Arc, time::Duration};
 
@@ -26,6 +26,8 @@ async fn ds_proxy_transformer() {
     let provider = Provider::<Http>::try_from(ganache.endpoint())
         .unwrap()
         .interval(Duration::from_millis(10u64));
+    let chain_id = provider.get_chainid().await.unwrap().as_u64();
+    let wallet = wallet.with_chain_id(chain_id);
     let signer_middleware = SignerMiddleware::new(provider.clone(), wallet);
     let wallet_addr = signer_middleware.address();
     let provider = Arc::new(signer_middleware.clone());
@@ -111,6 +113,8 @@ async fn ds_proxy_code() {
     let provider = Provider::<Http>::try_from(ganache.endpoint())
         .unwrap()
         .interval(Duration::from_millis(10u64));
+    let chain_id = provider.get_chainid().await.unwrap().as_u64();
+    let wallet = wallet.with_chain_id(chain_id);
     let signer_middleware = SignerMiddleware::new(provider.clone(), wallet);
     let wallet_addr = signer_middleware.address();
     let provider = Arc::new(signer_middleware.clone());