Fix Pending Transactions and EIP-155 (#22)

* fix(provider): ensure the Pending transaction calls the waker to get polled again

* feat(core): allow setting the blocktime in ganache

* test(provider): move pending txs test to integration tests + use block time

* fix(signers): make EIP-155 optional and fix sighash generation bug

Cargo.lock

@@ -341,6 +341,7 @@ dependencies = [
  "bincode",
  "ethabi",
  "ethereum-types",
+ "ethers",
  "glob",
  "libsecp256k1",
  "rand",
@@ -357,6 +358,7 @@ name = "ethers-providers"
 version = "0.1.0"
 dependencies = [
  "async-trait",
+ "ethers",
  "ethers-core",
  "futures-core",
  "futures-util",
@@ -374,6 +376,7 @@ dependencies = [
 name = "ethers-signers"
 version = "0.1.0"
 dependencies = [
+ "ethers",
  "ethers-core",
  "ethers-providers",
  "futures-util",

ethers-core/Cargo.toml

@@ -25,6 +25,7 @@ arrayvec = { version = "0.5.1", default-features = false, optional = true }
 glob = "0.3.0"
 
 [dev-dependencies]
+ethers = { version = "0.1.0", path = "../ethers" }
 serde_json = { version = "1.0.53", default-features = false }
 bincode = "1.2.1"
 

ethers-core/src/types/chainstate/transaction.rs

@@ -8,6 +8,12 @@ use rlp::RlpStream;
 use serde::{Deserialize, Serialize};
 use std::str::FromStr;
 
+// Number of tx fields before signing
+const UNSIGNED_TX_FIELDS: usize = 6;
+
+// Unsigned fields + signature [r s v]
+const SIGNED_TX_FIELDS: usize = UNSIGNED_TX_FIELDS + 3;
+
 /// Parameters for sending a transaction
 #[derive(Clone, Default, Serialize, Deserialize, PartialEq, Eq, Debug)]
 pub struct TransactionRequest {
@@ -113,14 +119,30 @@ impl TransactionRequest {
     }
 
     /// Hashes the transaction's data with the provided chain id
-    pub fn hash<T: Into<U64>>(&self, chain_id: Option<T>) -> H256 {
+    pub fn sighash<T: Into<U64>>(&self, chain_id: Option<T>) -> H256 {
         let mut rlp = RlpStream::new();
-        rlp.begin_list(9);
+        // "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);
         self.rlp_base(&mut rlp);
 
-        rlp.append(&chain_id.map(|c| c.into()).unwrap_or_else(U64::zero));
+        // Only hash the 3 extra fields when preparing the
-        rlp.append(&0u8);
+        // data to sign if chain_id is present
-        rlp.append(&0u8);
+        if let Some(chain_id) = chain_id {
+            rlp.append(&chain_id.into());
+            rlp.append(&0u8);
+            rlp.append(&0u8);
+        }
 
         keccak256(rlp.out().as_ref()).into()
     }
@@ -128,7 +150,7 @@ impl TransactionRequest {
     /// Produces the RLP encoding of the transaction with the provided signature
     pub fn rlp_signed(&self, signature: &Signature) -> Bytes {
         let mut rlp = RlpStream::new();
-        rlp.begin_list(9);
+        rlp.begin_list(SIGNED_TX_FIELDS);
         self.rlp_base(&mut rlp);
 
         rlp.append(&signature.v);
@@ -217,7 +239,7 @@ impl Transaction {
 
     pub fn rlp(&self) -> Bytes {
         let mut rlp = RlpStream::new();
-        rlp.begin_list(9);
+        rlp.begin_list(SIGNED_TX_FIELDS);
         rlp.append(&self.nonce);
         rlp.append(&self.gas_price);
         rlp.append(&self.gas);

ethers-core/src/types/crypto/keys.rs

@@ -120,13 +120,15 @@ impl PrivateKey {
         let gas_price = tx.gas_price.ok_or(TxError::GasPriceMissing)?;
         let gas = tx.gas.ok_or(TxError::GasMissing)?;
 
-        // Hash the transaction's RLP encoding
+        // Get the transaction's sighash
-        let hash = tx.hash(chain_id);
+        let sighash = tx.sighash(chain_id);
         let message =
-            Message::parse_slice(hash.as_bytes()).expect("hash is non-zero 32-bytes; qed");
+            Message::parse_slice(sighash.as_bytes()).expect("hash is non-zero 32-bytes; qed");
 
         // Sign it (with replay protection if applicable)
         let signature = self.sign_with_eip155(&message, chain_id);
+
+        // Get the actual transaction hash
         let rlp = tx.rlp_signed(&signature);
         let hash = keccak256(&rlp.0);
 

ethers-core/src/utils/ganache.rs

@@ -25,7 +25,7 @@ impl Drop for GanacheInstance {
 /// # Example
 ///
 /// ```no_run
-/// use ethers_core::utils::Ganache;
+/// use ethers::utils::Ganache;
 ///
 /// let port = 8545u64;
 /// let url = format!("http://localhost:{}", port).to_string();
@@ -40,6 +40,7 @@ impl Drop for GanacheInstance {
 #[derive(Clone, Default)]
 pub struct Ganache {
     port: Option<u64>,
+    block_time: Option<u64>,
     mnemonic: Option<String>,
 }
 
@@ -62,6 +63,12 @@ impl Ganache {
         self
     }
 
+    /// Sets the block-time which will be used when the `ganache-cli` instance is launched.
+    pub fn block_time<T: Into<u64>>(mut self, block_time: T) -> Self {
+        self.block_time = Some(block_time.into());
+        self
+    }
+
     /// Consumes the builder and spawns `ganache-cli` with stdout redirected
     /// to /dev/null. This takes ~2 seconds to execute as it blocks while
     /// waiting for `ganache-cli` to launch.
@@ -76,6 +83,10 @@ impl Ganache {
             cmd.arg("-m").arg(mnemonic);
         }
 
+        if let Some(block_time) = self.block_time {
+            cmd.arg("-b").arg(block_time.to_string());
+        }
+
         let ganache_pid = cmd.spawn().expect("couldnt start ganache-cli");
 
         // wait a couple of seconds for ganache to boot up

ethers-core/src/utils/mod.rs

@@ -13,16 +13,17 @@ pub use hash::{hash_message, id, keccak256, serialize};
 pub use rlp;
 
 use crate::types::{Address, Bytes, U256};
+use std::convert::TryInto;
 
 /// 1 Ether = 1e18 Wei
-pub const WEI: usize = 1000000000000000000;
+pub const WEI_IN_ETHER: usize = 1000000000000000000;
 
 /// Format the output for the user which prefer to see values
 /// in ether (instead of wei)
 ///
 /// Divides the input by 1e18
 pub fn format_ether<T: Into<U256>>(amount: T) -> U256 {
-    amount.into() / WEI
+    amount.into() / WEI_IN_ETHER
 }
 
 /// Divides with the number of decimals
@@ -30,16 +31,28 @@ pub fn format_units<T: Into<U256>>(amount: T, decimals: usize) -> U256 {
     amount.into() / decimals
 }
 
-/// Converts a string to a U256 and converts from Ether to Wei.
+/// Converts the input to a U256 and converts from Ether to Wei.
 ///
-/// Multiplies the input by 1e18
+/// ```
-pub fn parse_ether(eth: &str) -> Result<U256, rustc_hex::FromHexError> {
+/// use ethers::{types::U256, utils::{parse_ether, WEI_IN_ETHER}};
-    Ok(eth.parse::<U256>()? * WEI)
+///
+/// let eth = U256::from(WEI_IN_ETHER);
+/// assert_eq!(eth, parse_ether(1u8).unwrap());
+/// assert_eq!(eth, parse_ether(1usize).unwrap());
+/// assert_eq!(eth, parse_ether("1").unwrap());
+pub fn parse_ether<S>(eth: S) -> Result<U256, S::Error>
+where
+    S: TryInto<U256>,
+{
+    Ok(eth.try_into()? * WEI_IN_ETHER)
 }
 
 /// Multiplies with the number of decimals
-pub fn parse_units(eth: &str, decimals: usize) -> Result<U256, rustc_hex::FromHexError> {
+pub fn parse_units<S>(eth: S, decimals: usize) -> Result<U256, S::Error>
-    Ok(eth.parse::<U256>()? * decimals)
+where
+    S: TryInto<U256>,
+{
+    Ok(eth.try_into()? * decimals)
 }
 
 /// The address for an Ethereum contract is deterministically computed from the

ethers-core/src/utils/solc.rs

@@ -39,7 +39,7 @@ pub struct CompiledContract {
 /// # Examples
 ///
 /// ```no_run
-/// use ethers_core::utils::Solc;
+/// use ethers::utils::Solc;
 ///
 /// # fn main() -> Result<(), Box<dyn std::error::Error>> {
 /// // Give it a glob

ethers-providers/Cargo.toml

@@ -21,5 +21,7 @@ pin-project = { version = "0.4.20", default-features = false }
 tokio = { version = "0.2.21", default-features = false, features = ["time"] }
 
 [dev-dependencies]
+ethers = { version = "0.1.0", path = "../ethers" }
+
 rustc-hex = "2.1.0"
 tokio = { version = "0.2.21", default-features = false, features = ["rt-core", "macros"] }

ethers-providers/src/http.rs

@@ -17,8 +17,7 @@ use url::Url;
 /// # Example
 ///
 /// ```no_run
-/// use ethers_providers::{JsonRpcClient, Http};
+/// use ethers::{types::U64, providers::{JsonRpcClient, Http}};
-/// use ethers_core::types::U64;
 /// use std::str::FromStr;
 ///
 /// # async fn foo() -> Result<(), Box<dyn std::error::Error>> {
@@ -85,7 +84,7 @@ impl Provider {
     /// # Example
     ///
     /// ```
-    /// use ethers_providers::Http;
+    /// use ethers::providers::Http;
     /// use url::Url;
     ///
     /// let url = Url::parse("http://localhost:8545").unwrap();

ethers-providers/src/pending_transaction.rs

@@ -50,8 +50,12 @@ impl<'a, P: JsonRpcClient> Future for PendingTransaction<'a, P> {
 
         match this.state {
             PendingTxState::GettingReceipt(fut) => {
-                let receipt = futures_util::ready!(fut.as_mut().poll(ctx))?;
+                if let Ok(receipt) = futures_util::ready!(fut.as_mut().poll(ctx)) {
-                *this.state = PendingTxState::CheckingReceipt(Box::new(receipt))
+                    *this.state = PendingTxState::CheckingReceipt(Box::new(receipt))
+                } else {
+                    let fut = Box::pin(this.provider.get_transaction_receipt(*this.tx_hash));
+                    *this.state = PendingTxState::GettingReceipt(fut)
+                }
             }
             PendingTxState::CheckingReceipt(receipt) => {
                 // If we requested more than 1 confirmation, we need to compare the receipt's
@@ -59,7 +63,10 @@ impl<'a, P: JsonRpcClient> Future for PendingTransaction<'a, P> {
                 if *this.confirmations > 1 {
                     let fut = Box::pin(this.provider.get_block_number());
                     *this.state =
-                        PendingTxState::GettingBlockNumber(fut, Box::new(*receipt.clone()))
+                        PendingTxState::GettingBlockNumber(fut, Box::new(*receipt.clone()));
+
+                    // Schedule the waker to poll again
+                    ctx.waker().wake_by_ref();
                 } else {
                     let receipt = *receipt.clone();
                     *this.state = PendingTxState::Completed;
@@ -162,30 +169,3 @@ impl<'a> fmt::Debug for PendingTxState<'a> {
             .finish()
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use crate::Http;
-    use ethers_core::{types::TransactionRequest, utils::Ganache};
-    use std::convert::TryFrom;
-
-    #[tokio::test]
-    async fn test_pending_tx() {
-        let _ganache = Ganache::new().spawn();
-        let provider = Provider::<Http>::try_from("http://localhost:8545").unwrap();
-        let accounts = provider.get_accounts().await.unwrap();
-        let tx = TransactionRequest::pay(accounts[0], 1000).from(accounts[0]);
-
-        let pending_tx = provider.send_transaction(tx).await.unwrap();
-
-        let receipt = provider
-            .get_transaction_receipt(pending_tx.tx_hash)
-            .await
-            .unwrap();
-
-        // the pending tx resolves to the same receipt
-        let tx_receipt = pending_tx.confirmations(1).await.unwrap();
-        assert_eq!(receipt, tx_receipt);
-    }
-}

ethers-providers/src/provider.rs

@@ -28,8 +28,7 @@ use std::{convert::TryFrom, fmt::Debug};
 /// # Example
 ///
 /// ```no_run
-/// # use ethers_providers::JsonRpcClient;
+/// use ethers::providers::{JsonRpcClient, Provider, Http};
-/// use ethers_providers::{Provider, Http};
 /// use std::convert::TryFrom;
 ///
 /// let provider = Provider::<Http>::try_from(

ethers-providers/tests/provider.rs

@@ -0,0 +1,21 @@
+use ethers::{
+    providers::{Http, Provider},
+    types::TransactionRequest,
+    utils::{parse_ether, Ganache},
+};
+use std::convert::TryFrom;
+
+#[tokio::test]
+async fn pending_txs_with_confirmations_ganache() {
+    let _ganache = Ganache::new().block_time(2u64).spawn();
+    let provider = Provider::<Http>::try_from("http://localhost:8545").unwrap();
+    let accounts = provider.get_accounts().await.unwrap();
+
+    let tx = TransactionRequest::pay(accounts[1], parse_ether(1u64).unwrap()).from(accounts[0]);
+    let pending_tx = provider.send_transaction(tx).await.unwrap();
+    let hash = *pending_tx;
+    let receipt = pending_tx.confirmations(5).await.unwrap();
+
+    // got the correct receipt
+    assert_eq!(receipt.transaction_hash, hash);
+}

ethers-signers/Cargo.toml

@@ -12,4 +12,6 @@ futures-util = { version = "0.3.5", default-features = false }
 serde = "1.0.112"
 
 [dev-dependencies]
+ethers = { version = "0.1.0", path = "../ethers" }
+
 tokio = { version = "0.2.21", features = ["macros"] }

ethers-signers/src/wallet.rs

@@ -72,7 +72,7 @@ pub struct Wallet {
     /// The wallet's address
     address: Address,
     /// The wallet's chain id (for EIP-155), signs w/o replay protection if left unset
-    chain_id: u64,
+    chain_id: Option<u64>,
 }
 
 impl Signer for Wallet {
@@ -83,7 +83,7 @@ impl Signer for Wallet {
     }
 
     fn sign_transaction(&self, tx: TransactionRequest) -> Result<Transaction, Self::Error> {
-        self.private_key.sign_transaction(tx, Some(self.chain_id))
+        self.private_key.sign_transaction(tx, self.chain_id)
     }
 
     fn address(&self) -> Address {
@@ -110,7 +110,7 @@ impl Wallet {
             private_key,
             public_key,
             address,
-            chain_id: 1,
+            chain_id: None,
         }
     }
 
@@ -126,7 +126,7 @@ impl Wallet {
 
     /// Sets the wallet's chain_id, used in conjunction with EIP-155 signing
     pub fn set_chain_id<T: Into<u64>>(mut self, chain_id: T) -> Self {
-        self.chain_id = chain_id.into();
+        self.chain_id = Some(chain_id.into());
         self
     }
 
@@ -141,7 +141,7 @@ impl Wallet {
     }
 
     /// Gets the wallet's chain id
-    pub fn chain_id(&self) -> u64 {
+    pub fn chain_id(&self) -> Option<u64> {
         self.chain_id
     }
 }
@@ -155,7 +155,7 @@ impl From<PrivateKey> for Wallet {
             private_key,
             public_key,
             address,
-            chain_id: 1,
+            chain_id: None,
         }
     }
 }

ethers-signers/tests/send_eth.rs

@@ -1,8 +1,34 @@
-use ethers_core::{types::TransactionRequest, utils::Ganache};
+use ethers::{
-use ethers_providers::{Http, Provider};
+    providers::{Http, Provider},
-use ethers_signers::Wallet;
+    signers::Wallet,
+    types::TransactionRequest,
+    utils::{parse_ether, Ganache},
+};
 use std::convert::TryFrom;
 
+#[tokio::test]
+async fn pending_txs_with_confirmations_rinkeby_infura() {
+    let provider =
+        Provider::<Http>::try_from("https://rinkeby.infura.io/v3/c60b0bb42f8a4c6481ecd229eddaca27")
+            .unwrap();
+
+    // pls do not drain this key :)
+    // note: this works even if there's no EIP-155 configured!
+    let client = "FF7F80C6E9941865266ED1F481263D780169F1D98269C51167D20C630A5FDC8A"
+        .parse::<Wallet>()
+        .unwrap()
+        .connect(provider);
+
+    let tx = TransactionRequest::pay(client.address(), parse_ether(1u64).unwrap());
+    let pending_tx = client.send_transaction(tx, None).await.unwrap();
+    let hash = *pending_tx;
+    dbg!(hash);
+    let receipt = pending_tx.confirmations(3).await.unwrap();
+
+    // got the correct receipt
+    assert_eq!(receipt.transaction_hash, hash);
+}
+
 #[tokio::test]
 async fn send_eth() {
     let port = 8545u64;