Skip to content

Commit

Permalink
Work around fork the issue with Geth nodes sometimes returning wrong …
Browse files Browse the repository at this point in the history
…"status" for the transactions

(ethereum/go-ethereum#24124).
Unit tests should pass and be equivalent now for both JSON-RPC and GraphQL access.
  • Loading branch information
amyodov committed Dec 16, 2021
1 parent 7915b86 commit ed5e080
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
}

Expand Down Expand Up @@ -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
(
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand All @@ -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,
Expand Down Expand Up @@ -345,7 +349,7 @@ abstract class AbstractEthereumNodeConnectorSpec extends AnyFlatSpec {
)
)(
sharedConnector.readBlock(
blockNumber = 10381084,
blockNumber = 10_381_084,
addressesOfInterest = Set("0x3452519f4711703e13ea0863487eb8401bd6ae57"))
)
}
Expand Down

0 comments on commit ed5e080

Please sign in to comment.