Fix gas_used fields in receipts (paritytech#261)

* gas_used should be cumulative_gas_used!!!

* more runtime traces

* improve logs

bridges/modules/currency-exchange/src/lib.rs

@@ -147,7 +147,13 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
 	/// Returns true if currency exchange module is able to import given transaction proof in
 	/// its current state.
 	pub fn filter_transaction_proof(proof: &<T::PeerBlockchain as PeerBlockchain>::TransactionInclusionProof) -> bool {
-		if prepare_deposit_details::<T, I>(proof).is_err() {
+		if let Err(err) = prepare_deposit_details::<T, I>(proof) {
+			frame_support::debug::trace!(
+				target: "runtime",
+				"Can't accept exchange transaction: {:?}",
+				err,
+			);
+
 			return false;
 		}
 

bridges/modules/ethereum/src/lib.rs

@@ -893,17 +893,40 @@ pub fn verify_transaction_finalized<S: Storage>(
 	proof: &[(RawTransaction, RawTransactionReceipt)],
 ) -> bool {
 	if tx_index >= proof.len() as _ {
+		frame_support::debug::trace!(
+			target: "runtime",
+			"Tx finality check failed: transaction index ({}) is larger than number of transactions ({})",
+			tx_index,
+			proof.len(),
+		);
+
 		return false;
 	}
 
 	let header = match storage.header(&block) {
 		Some((header, _)) => header,
-		None => return false,
+		None => {
+			frame_support::debug::trace!(
+				target: "runtime",
+				"Tx finality check failed: can't find header in the storage: {}",
+				block,
+			);
+
+			return false;
+		}
 	};
 	let finalized = storage.finalized_block();
 
 	// if header is not yet finalized => return
 	if header.number > finalized.number {
+		frame_support::debug::trace!(
+			target: "runtime",
+			"Tx finality check failed: header {}/{} is not finalized. Best finalized: {}",
+			header.number,
+			block,
+			finalized.number,
+		);
+
 		return false;
 	}
 
@@ -915,24 +938,62 @@ pub fn verify_transaction_finalized<S: Storage>(
 		false => block == finalized.hash,
 	};
 	if !is_finalized {
+		frame_support::debug::trace!(
+			target: "runtime",
+			"Tx finality check failed: header {} is not finalized: no canonical path to best finalized block {}",
+			block,
+			finalized.hash,
+		);
+
 		return false;
 	}
 
 	// verify that transaction is included in the block
-	if !header.verify_transactions_root(proof.iter().map(|(tx, _)| tx)) {
+	if let Err(computed_root) = header.check_transactions_root(proof.iter().map(|(tx, _)| tx)) {
+		frame_support::debug::trace!(
+			target: "runtime",
+			"Tx finality check failed: transactions root mismatch. Expected: {}, computed: {}",
+			header.transactions_root,
+			computed_root,
+		);
+
 		return false;
 	}
 
 	// verify that transaction receipt is included in the block
-	if !header.verify_raw_receipts_root(proof.iter().map(|(_, r)| r)) {
+	if let Err(computed_root) = header.check_raw_receipts_root(proof.iter().map(|(_, r)| r)) {
+		frame_support::debug::trace!(
+			target: "runtime",
+			"Tx finality check failed: receipts root mismatch. Expected: {}, computed: {}",
+			header.receipts_root,
+			computed_root,
+		);
+
 		return false;
 	}
 
 	// check that transaction has completed successfully
-	matches!(
-		Receipt::is_successful_raw_receipt(&proof[tx_index as usize].1),
-		Ok(true)
-	)
+	let is_successful_raw_receipt = Receipt::is_successful_raw_receipt(&proof[tx_index as usize].1);
+	match is_successful_raw_receipt {
+		Ok(true) => true,
+		Ok(false) => {
+			frame_support::debug::trace!(
+				target: "runtime",
+				"Tx finality check failed: receipt shows that transaction has failed",
+			);
+
+			false
+		}
+		Err(err) => {
+			frame_support::debug::trace!(
+				target: "runtime",
+				"Tx finality check failed: receipt check has failed: {}",
+				err,
+			);
+
+			false
+		}
+	}
 }
 
 /// Transaction pool configuration.

bridges/modules/ethereum/src/validators.rs

@@ -132,7 +132,7 @@ impl<'a> Validators<'a> {
 		}
 
 		let receipts = receipts.ok_or(Error::MissingTransactionsReceipts)?;
-		if !header.verify_receipts_root(&receipts) {
+		if header.check_receipts_root(&receipts).is_err() {
 			return Err(Error::TransactionsReceiptsMismatch);
 		}
 

bridges/modules/ethereum/src/verification.rs

@@ -144,7 +144,7 @@ pub fn accept_aura_header_into_pool<S: Storage>(
 	// the heaviest, but rare operation - we do not want invalid receipts in the pool
 	if let Some(receipts) = receipts {
 		frame_support::debug::trace!(target: "runtime", "Got receipts! {:?}", receipts);
-		if !header.verify_receipts_root(receipts) {
+		if header.check_receipts_root(receipts).is_err() {
 			return Err(Error::TransactionsReceiptsMismatch);
 		}
 	}

bridges/primitives/ethereum-poa/src/lib.rs

@@ -205,18 +205,30 @@ impl Header {
 	}
 
 	/// Check if passed transactions receipts are matching receipts root in this header.
-	pub fn verify_receipts_root(&self, receipts: &[Receipt]) -> bool {
-		verify_merkle_proof(self.receipts_root, receipts.iter().map(|r| r.rlp()))
+	/// Returns Ok(computed-root) if check succeeds.
+	/// Returns Err(computed-root) if check fails.
+	pub fn check_receipts_root(&self, receipts: &[Receipt]) -> Result<H256, H256> {
+		check_merkle_proof(self.receipts_root, receipts.iter().map(|r| r.rlp()))
 	}
 
 	/// Check if passed raw transactions receipts are matching receipts root in this header.
-	pub fn verify_raw_receipts_root<'a>(&self, receipts: impl IntoIterator<Item = &'a RawTransactionReceipt>) -> bool {
-		verify_merkle_proof(self.receipts_root, receipts.into_iter())
+	/// Returns Ok(computed-root) if check succeeds.
+	/// Returns Err(computed-root) if check fails.
+	pub fn check_raw_receipts_root<'a>(
+		&self,
+		receipts: impl IntoIterator<Item = &'a RawTransactionReceipt>,
+	) -> Result<H256, H256> {
+		check_merkle_proof(self.receipts_root, receipts.into_iter())
 	}
 
 	/// Check if passed transactions are matching transactions root in this header.
-	pub fn verify_transactions_root<'a>(&self, transactions: impl IntoIterator<Item = &'a RawTransaction>) -> bool {
-		verify_merkle_proof(self.transactions_root, transactions.into_iter())
+	/// Returns Ok(computed-root) if check succeeds.
+	/// Returns Err(computed-root) if check fails.
+	pub fn check_transactions_root<'a>(
+		&self,
+		transactions: impl IntoIterator<Item = &'a RawTransaction>,
+	) -> Result<H256, H256> {
+		check_merkle_proof(self.transactions_root, transactions.into_iter())
 	}
 
 	/// Gets the seal hash of this header.
@@ -502,9 +514,16 @@ pub fn public_to_address(public: &[u8; 64]) -> Address {
 	result
 }
 
-/// Verify ethereum merkle proof.
-fn verify_merkle_proof<T: AsRef<[u8]>>(expected_root: H256, items: impl Iterator<Item = T>) -> bool {
-	compute_merkle_root(items) == expected_root
+/// Check ethereum merkle proof.
+/// Returns Ok(computed-root) if check succeeds.
+/// Returns Err(computed-root) if check fails.
+fn check_merkle_proof<T: AsRef<[u8]>>(expected_root: H256, items: impl Iterator<Item = T>) -> Result<H256, H256> {
+	let computed_root = compute_merkle_root(items);
+	if computed_root == expected_root {
+		Ok(computed_root)
+	} else {
+		Err(computed_root)
+	}
 }
 
 /// Compute ethereum merkle root.

bridges/relays/ethereum/src/ethereum_client.rs

@@ -169,12 +169,7 @@ impl EthereumRpc for EthereumRpcClient {
 	}
 
 	async fn transaction_receipt(&self, transaction_hash: H256) -> Result<Receipt> {
-		let receipt = Ethereum::get_transaction_receipt(&self.client, transaction_hash).await?;
-
-		match receipt.gas_used {
-			Some(_) => Ok(receipt),
-			None => Err(RpcError::Ethereum(EthereumNodeError::IncompleteReceipt)),
-		}
+		Ok(Ethereum::get_transaction_receipt(&self.client, transaction_hash).await?)
 	}
 
 	async fn account_nonce(&self, address: Address) -> Result<U256> {

bridges/relays/ethereum/src/ethereum_types.rs

@@ -24,10 +24,6 @@ pub use web3::types::{Address, Bytes, CallRequest, H256, U128, U256, U64};
 /// both number and hash fields filled.
 pub const HEADER_ID_PROOF: &str = "checked on retrieval; qed";
 
-/// When receipt is just received from the Ethereum node, we check that it has
-/// gas_used field filled.
-pub const RECEIPT_GAS_USED_PROOF: &str = "checked on retrieval; qed";
-
 /// Ethereum transaction hash type.
 pub type TransactionHash = H256;
 

bridges/relays/ethereum/src/rpc_errors.rs

@@ -95,8 +95,6 @@ pub enum EthereumNodeError {
 	ResponseParseFailed(String),
 	/// We have received a header with missing fields.
 	IncompleteHeader,
-	/// We have received a receipt missing a `gas_used` field.
-	IncompleteReceipt,
 	/// We have received a transaction missing a `raw` field.
 	IncompleteTransaction,
 	/// An invalid Substrate block number was received from
@@ -112,9 +110,6 @@ impl ToString for EthereumNodeError {
 				"Incomplete Ethereum Header Received (missing some of required fields - hash, number, logs_bloom)"
 					.to_string()
 			}
-			Self::IncompleteReceipt => {
-				"Incomplete Ethereum Receipt Recieved (missing required field - gas_used)".to_string()
-			}
 			Self::IncompleteTransaction => "Incomplete Ethereum Transaction (missing required field - raw)".to_string(),
 			Self::InvalidSubstrateBlockNumber => "Received an invalid Substrate block from Ethereum Node".to_string(),
 		}

bridges/relays/ethereum/src/substrate_types.rs

@@ -16,7 +16,6 @@
 
 use crate::ethereum_types::{
 	Header as EthereumHeader, Receipt as EthereumReceipt, HEADER_ID_PROOF as ETHEREUM_HEADER_ID_PROOF,
-	RECEIPT_GAS_USED_PROOF as ETHEREUM_RECEIPT_GAS_USED_PROOF,
 };
 use crate::sync_types::{HeaderId, HeadersSyncPipeline, QueuedHeader, SourceHeader};
 use codec::Encode;
@@ -108,7 +107,7 @@ pub fn into_substrate_ethereum_receipts(
 /// Convert Ethereum transactions receipt into Ethereum transactions receipt for Substrate.
 pub fn into_substrate_ethereum_receipt(receipt: &EthereumReceipt) -> SubstrateEthereumReceipt {
 	SubstrateEthereumReceipt {
-		gas_used: receipt.gas_used.expect(ETHEREUM_RECEIPT_GAS_USED_PROOF),
+		gas_used: receipt.cumulative_gas_used,
 		log_bloom: receipt.logs_bloom.data().into(),
 		logs: receipt
 			.logs