From ed5e080b73d8ec0cdd1b63b7009f7ff3e7279025 Mon Sep 17 00:00:00 2001 From: Alex Myodov Date: Thu, 16 Dec 2021 22:14:57 +0200 Subject: [PATCH] Work around fork the issue with Geth nodes sometimes returning wrong "status" for the transactions (https://github.com/ethereum/go-ethereum/issues/24124). Unit tests should pass and be equivalent now for both JSON-RPC and GraphQL access. --- .../unicherrygarden/ethereum/EthUtils.java | 2 + .../EthereumSingleNodeGraphQLConnector.scala | 17 +++- .../EthereumSingleNodeJsonRpcConnector.scala | 7 +- .../AbstractEthereumNodeConnectorSpec.scala | 78 ++++++++++--------- 4 files changed, 61 insertions(+), 43 deletions(-) diff --git a/java/ethutils/src/main/java/com/myodov/unicherrygarden/ethereum/EthUtils.java b/java/ethutils/src/main/java/com/myodov/unicherrygarden/ethereum/EthUtils.java index 8e7c9a0f..76ca6791 100644 --- a/java/ethutils/src/main/java/com/myodov/unicherrygarden/ethereum/EthUtils.java +++ b/java/ethutils/src/main/java/com/myodov/unicherrygarden/ethereum/EthUtils.java @@ -12,6 +12,8 @@ public class EthUtils { static final int TRANSACTION_HASH_LENGTH = 66; static final int ADDRESS_HASH_LENGTH = 42; + public static final int BYZANTIUM_FIRST_BLOCK = 4370000; + /** * Check if a string is a valid “hex string”, like ones used to store hashes. diff --git a/scala/connectors/ethereum_rpc_connector/src/main/scala/com/myodov/unicherrygarden/connectors/EthereumSingleNodeGraphQLConnector.scala b/scala/connectors/ethereum_rpc_connector/src/main/scala/com/myodov/unicherrygarden/connectors/EthereumSingleNodeGraphQLConnector.scala index eae5ce43..1b374f72 100644 --- a/scala/connectors/ethereum_rpc_connector/src/main/scala/com/myodov/unicherrygarden/connectors/EthereumSingleNodeGraphQLConnector.scala +++ b/scala/connectors/ethereum_rpc_connector/src/main/scala/com/myodov/unicherrygarden/connectors/EthereumSingleNodeGraphQLConnector.scala @@ -6,6 +6,7 @@ import akka.actor.ActorSystem import caliban.client.CalibanClientError import com.myodov.unicherrygarden.api.dlt import com.myodov.unicherrygarden.connectors.graphql.{BlockBasic, BlockBasicView, TransactionFullView} +import com.myodov.unicherrygarden.ethereum.EthUtils import com.typesafe.scalalogging.LazyLogging import sttp.capabilities import sttp.capabilities.akka.AkkaStreams @@ -131,8 +132,10 @@ class EthereumSingleNodeGraphQLConnector(nodeUrl: String, ) } + val blockNumber = Math.toIntExact(blockBasic.number) + val block = dlt.EthereumBlock( - number = blockBasic.number.toInt, + number = blockNumber, hash = blockBasic.hash, parentHash = blockBasic.parent match { // We need some custom handling of parent @@ -156,8 +159,16 @@ class EthereumSingleNodeGraphQLConnector(nodeUrl: String, value = tr.value, // *** Mined transaction *** // "status" – EIP 658, since Byzantium fork - // using Option(nullable) - status = tr.status.map(Math.toIntExact), // Option[Long] to Option[Int] + // using Option(nullable). + // But there seems to be a bug in GraphQL handling pre-Byzantium statuses, + // (https://github.com/ethereum/go-ethereum/issues/24124) + // So need to handle this manually. + status = blockNumber match { + case preByzantium if preByzantium < EthUtils.BYZANTIUM_FIRST_BLOCK => + None + case byzantiumAndNewer => + tr.status.map(Math.toIntExact) // Option[Long] to Option[Int] + }, blockNumber = tr.block.get.number, // block must exist! transactionIndex = tr.index.get, // transaction must exist! gasUsed = tr.gasUsed.get, // presumed non-null if mined diff --git a/scala/connectors/ethereum_rpc_connector/src/main/scala/com/myodov/unicherrygarden/connectors/EthereumSingleNodeJsonRpcConnector.scala b/scala/connectors/ethereum_rpc_connector/src/main/scala/com/myodov/unicherrygarden/connectors/EthereumSingleNodeJsonRpcConnector.scala index 6321c1ee..9e2d2419 100644 --- a/scala/connectors/ethereum_rpc_connector/src/main/scala/com/myodov/unicherrygarden/connectors/EthereumSingleNodeJsonRpcConnector.scala +++ b/scala/connectors/ethereum_rpc_connector/src/main/scala/com/myodov/unicherrygarden/connectors/EthereumSingleNodeJsonRpcConnector.scala @@ -164,11 +164,12 @@ class EthereumSingleNodeJsonRpcConnector(nodeUrl: String) readBlockWeb3j(blockNumber) match { case None => None case Some((w3jBlock, w3jTransactions, w3jReceiptsByTrHash)) => { - assert(blockNumber.bigInteger == w3jBlock.getNumber, + require(blockNumber.bigInteger == w3jBlock.getNumber, (blockNumber, w3jBlock.getNumber)) - assert(w3jBlock.getTransactions.size == w3jTransactions.size, + // These may easily fail if the node is not fully synced + require(w3jBlock.getTransactions.size == w3jTransactions.size, (blockNumber, w3jBlock.getTransactions.size, w3jTransactions.size)) - assert(w3jTransactions.size == w3jReceiptsByTrHash.size, + require(w3jTransactions.size == w3jReceiptsByTrHash.size, (blockNumber, w3jTransactions.size, w3jReceiptsByTrHash.size)) val blockHash = w3jBlock.getHash diff --git a/scala/connectors/ethereum_rpc_connector/src/test/scala/com/myodov/unicherrygarden/connectors/AbstractEthereumNodeConnectorSpec.scala b/scala/connectors/ethereum_rpc_connector/src/test/scala/com/myodov/unicherrygarden/connectors/AbstractEthereumNodeConnectorSpec.scala index e375ac46..e0a15957 100644 --- a/scala/connectors/ethereum_rpc_connector/src/test/scala/com/myodov/unicherrygarden/connectors/AbstractEthereumNodeConnectorSpec.scala +++ b/scala/connectors/ethereum_rpc_connector/src/test/scala/com/myodov/unicherrygarden/connectors/AbstractEthereumNodeConnectorSpec.scala @@ -84,39 +84,43 @@ abstract class AbstractEthereumNodeConnectorSpec extends AnyFlatSpec { // Blocks 1264, 120522 and 120545 are the only blocks for address 0x90d331f19e4ef54c4dc2710087ebd8536084a85a // In block 1264, it mined some ETH; in 120522 and 120545 it moved out some ETH // 60003 block has 4 transactions - "readBlock(120522)" should "parse block and find some single ETH transfer from address" in { - assertResult( - Some( // Option of Tuple - ( - EthereumBlock( - 120522, - hash = "0xace30c8603317cff2dd4ba7ddfce8e67bc94ea74ec5479ef1c802985551a1662", - parentHash = Some("0xd9d68173bb59563f20ec99fcce92dbc33c25160a1c77dd1257243b1fcf723003"), - Instant.parse("2015-08-21T12:48:31Z") - ), - List( - EthereumMinedTransaction( - txhash = "0x8fda3564427d18f119aa2309306babc7cd137893bd32260e4c75b9f74d3eeff6", - from = "0x90d331f19e4ef54c4dc2710087ebd8536084a85a", - to = Some("0x8f22398f1567cddaba1b6bb1973e62b4992d5c9c"), - nonce = 0, - value = BigInt(1000000000000000000l), - status = None, - blockNumber = BigInt(120522), - gas = BigInt(90000), - gasPrice = BigInt(59920793400l), - transactionIndex = 0, - gasUsed = BigInt(21000), - effectiveGasPrice = BigInt(59920793400l), - cumulativeGasUsed = BigInt(21000) - ) + "readBlock(120522) - pre-Byzantium" should "parse block and find some single ETH transfer from address" in { + val expected = Some( // Option of Tuple + ( + EthereumBlock( + 120522, + hash = "0xace30c8603317cff2dd4ba7ddfce8e67bc94ea74ec5479ef1c802985551a1662", + parentHash = Some("0xd9d68173bb59563f20ec99fcce92dbc33c25160a1c77dd1257243b1fcf723003"), + Instant.parse("2015-08-21T12:48:31Z") + ), + List( + EthereumMinedTransaction( + txhash = "0x8fda3564427d18f119aa2309306babc7cd137893bd32260e4c75b9f74d3eeff6", + from = "0x90d331f19e4ef54c4dc2710087ebd8536084a85a", + to = Some("0x8f22398f1567cddaba1b6bb1973e62b4992d5c9c"), + nonce = 0, + value = BigInt(1000000000000000000l), + status = None, + blockNumber = BigInt(120522), + gas = BigInt(90000), + gasPrice = BigInt(59920793400l), + transactionIndex = 0, + gasUsed = BigInt(21000), + effectiveGasPrice = BigInt(59920793400l), + cumulativeGasUsed = BigInt(21000) ) ) ) + ) + + val actual = sharedConnector.readBlock( + blockNumber = 120522, + addressesOfInterest = Set("0x90d331f19e4ef54c4dc2710087ebd8536084a85a")) + + assertResult( + expected )( - sharedConnector.readBlock( - blockNumber = 120522, - addressesOfInterest = Set("0x90d331f19e4ef54c4dc2710087ebd8536084a85a")) + actual ) } @@ -194,7 +198,7 @@ abstract class AbstractEthereumNodeConnectorSpec extends AnyFlatSpec { // 60003 block has 4 transactions - "readBlock(60003)" should "read and parse block (with some filters)" in { + "readBlock(60003) - pre-Byzantium" should "read and parse block (with some filters)" in { assertResult( Some( // Option of Tuple ( @@ -214,12 +218,12 @@ abstract class AbstractEthereumNodeConnectorSpec extends AnyFlatSpec { ) } - it should "read and parse some heavily used block (with some filters)" in { + "readBlock(11906373) - post-Byzantium" should "read and parse some heavily used block (with some filters)" in { assertResult( Some( // Option of Tuple ( EthereumBlock( - 11906373, + 11_906_373, hash = "0x71313d0f8edb2146c071d088a7ea4f91dd6f108ee42a7b7041c95a6154ed94e8", parentHash = Some("0x7a5412e1e68f2627ac671e33a0b8f1e0aad47231b78333328dabdaf5e1b692d9"), Instant.parse("2021-02-22T10:50:22Z") @@ -257,17 +261,17 @@ abstract class AbstractEthereumNodeConnectorSpec extends AnyFlatSpec { ) )( sharedConnector.readBlock( - blockNumber = 11906373, + blockNumber = 11_906_373, addressesOfInterest = Set("0x28bacfa4fc8b8ed6f50cbd0eb1d58bc508eb8e15")) ) } - "readBlock(10381084)" should "find a transaction generating (5) ERC20 Transfer events from smart contract" in { + "readBlock(10381084) - post-Byzantium" should "find a transaction generating (5) ERC20 Transfer events from smart contract" in { assertResult( Some( // Option of Tuple ( EthereumBlock( - 10381084, + 10_381_084, hash = "0x703aaebacb5abb97043cf22d806e0ff1b947b8f09673e8539b1abb78f737b707", parentHash = Some("0x3cee0f907ad3d303a32759d513eb538d58a56e2eb78985edb81841e707b1bde2"), Instant.parse("2020-07-02T16:30:14Z") @@ -280,7 +284,7 @@ abstract class AbstractEthereumNodeConnectorSpec extends AnyFlatSpec { nonce = 119, value = 0, status = Some(1), - blockNumber = BigInt(10381084l), + blockNumber = BigInt(10_381_084), gas = BigInt(5000000), gasPrice = BigInt(43000000000l), transactionIndex = 38, @@ -345,7 +349,7 @@ abstract class AbstractEthereumNodeConnectorSpec extends AnyFlatSpec { ) )( sharedConnector.readBlock( - blockNumber = 10381084, + blockNumber = 10_381_084, addressesOfInterest = Set("0x3452519f4711703e13ea0863487eb8401bd6ae57")) ) }