From 4c2251b749fd5f86ce420ce0b3c70005b9160ed2 Mon Sep 17 00:00:00 2001 From: Daniel Lehrner Date: Wed, 5 Oct 2022 18:23:38 +0200 Subject: [PATCH] Log index is counted per block, not per transaction (#4355) * Log index is counted per block, not per transaction Signed-off-by: Daniel Lehrner Co-authored-by: Sally MacFarlane Co-authored-by: mark-terry --- CHANGELOG.md | 1 + .../pojoadapter/TransactionAdapter.java | 22 ++- .../ethereum/api/query/BlockchainQueries.java | 132 ++++++++++++++---- .../ethereum/api/query/PrivacyQueries.java | 25 ++-- .../filter/FilterManagerLogFilterTest.java | 2 +- .../logs/LogsSubscriptionServiceTest.java | 37 +++-- .../besu/ethereum/core/LogWithMetadata.java | 29 +++- 7 files changed, 186 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4da5b2ab638..66c144f0b5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Add Mainnet to merged networks [#4463](https://github.com/hyperledger/besu/pull/4463) ### Bug Fixes +- Fixed logIndex value returned by eth_getLogs RPC call [#4355](https://github.com/hyperledger/besu/pull/4355) ### Download Links ## 22.7.4 diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/pojoadapter/TransactionAdapter.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/pojoadapter/TransactionAdapter.java index 64285264a9f..c08d04e6afd 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/pojoadapter/TransactionAdapter.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/pojoadapter/TransactionAdapter.java @@ -21,6 +21,7 @@ import org.hyperledger.besu.ethereum.api.query.BlockchainQueries; import org.hyperledger.besu.ethereum.api.query.TransactionReceiptWithMetadata; import org.hyperledger.besu.ethereum.api.query.TransactionWithMetadata; +import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.LogWithMetadata; import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.core.TransactionReceipt; @@ -168,18 +169,25 @@ public Optional getCreatedContract(final DataFetchingEnvironment public List getLogs(final DataFetchingEnvironment environment) { final BlockchainQueries query = getBlockchainQueries(environment); final Hash hash = transactionWithMetadata.getTransaction().getHash(); + + final Optional maybeBlockHeader = + transactionWithMetadata.getBlockNumber().flatMap(query::getBlockHeaderByNumber); + + if (maybeBlockHeader.isEmpty()) { + throw new RuntimeException( + "Cannot get block (" + + transactionWithMetadata.getBlockNumber() + + ") for transaction " + + transactionWithMetadata.getTransaction().getHash()); + } + final Optional maybeTransactionReceiptWithMetadata = query.transactionReceiptByTransactionHash(hash); final List results = new ArrayList<>(); if (maybeTransactionReceiptWithMetadata.isPresent()) { final List logs = - LogWithMetadata.generate( - maybeTransactionReceiptWithMetadata.get().getReceipt(), - transactionWithMetadata.getBlockNumber().get(), - transactionWithMetadata.getBlockHash().get(), - hash, - transactionWithMetadata.getTransactionIndex().get(), - false); + query.matchingLogs( + maybeBlockHeader.get().getBlockHash(), transactionWithMetadata, () -> true); for (final LogWithMetadata log : logs) { results.add(new LogAdapter(log)); } diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/BlockchainQueries.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/BlockchainQueries.java index 2461a8fed32..673882339ef 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/BlockchainQueries.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/BlockchainQueries.java @@ -47,6 +47,7 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -752,43 +753,33 @@ private List matchingLogsCached( public List matchingLogs( final Hash blockHash, final LogsQuery query, final Supplier isQueryAlive) { try { - final Optional blockHeader = - BackendQuery.runIfAlive( - "matchingLogs - getBlockHeader", - () -> blockchain.getBlockHeader(blockHash), - isQueryAlive); + final Optional blockHeader = getBlockHeader(blockHash, isQueryAlive); if (blockHeader.isEmpty()) { return Collections.emptyList(); } // receipts and transactions should exist if the header exists, so throwing is ok. - final List receipts = - BackendQuery.runIfAlive( - "matchingLogs - getTxReceipts", - () -> blockchain.getTxReceipts(blockHash).orElseThrow(), - isQueryAlive); - final List transactions = - BackendQuery.runIfAlive( - "matchingLogs - getBlockBody", - () -> blockchain.getBlockBody(blockHash).orElseThrow().getTransactions(), - isQueryAlive); + final List receipts = getReceipts(blockHash, isQueryAlive); + final List transactions = getTransactions(blockHash, isQueryAlive); final long number = blockHeader.get().getNumber(); - final boolean removed = - BackendQuery.runIfAlive( - "matchingLogs - blockIsOnCanonicalChain", - () -> !blockchain.blockIsOnCanonicalChain(blockHash), - isQueryAlive); + final boolean removed = getRemoved(blockHash, isQueryAlive); + + final AtomicInteger logIndexOffset = new AtomicInteger(); return IntStream.range(0, receipts.size()) .mapToObj( i -> { try { BackendQuery.stopIfExpired(isQueryAlive); - return LogWithMetadata.generate( - receipts.get(i), - number, - blockHash, - transactions.get(i).getHash(), - i, - removed); + final List result = + LogWithMetadata.generate( + logIndexOffset.intValue(), + receipts.get(i), + number, + blockHash, + transactions.get(i).getHash(), + i, + removed); + logIndexOffset.addAndGet(receipts.get(i).getLogs().size()); + return result; } catch (final Exception e) { throw new RuntimeException(e); } @@ -801,6 +792,47 @@ public List matchingLogs( } } + public List matchingLogs( + final Hash blockHash, + final TransactionWithMetadata transactionWithMetaData, + final Supplier isQueryAlive) { + if (transactionWithMetaData.getTransactionIndex().isEmpty()) { + throw new RuntimeException( + "Cannot find logs because transaction " + + transactionWithMetaData.getTransaction().getHash() + + " does not have a transaction index"); + } + + try { + final Optional blockHeader = getBlockHeader(blockHash, isQueryAlive); + if (blockHeader.isEmpty()) { + return Collections.emptyList(); + } + // receipts and transactions should exist if the header exists, so throwing is ok. + final List receipts = getReceipts(blockHash, isQueryAlive); + final List transactions = getTransactions(blockHash, isQueryAlive); + final long number = blockHeader.get().getNumber(); + final boolean removed = getRemoved(blockHash, isQueryAlive); + + final int transactionIndex = transactionWithMetaData.getTransactionIndex().get(); + final int logIndexOffset = + logIndexOffset( + transactionWithMetaData.getTransaction().getHash(), receipts, transactions); + + return LogWithMetadata.generate( + logIndexOffset, + receipts.get(transactionIndex), + number, + blockHash, + transactions.get(transactionIndex).getHash(), + transactionIndex, + removed); + + } catch (final Exception e) { + throw new RuntimeException(e); + } + } + /** * Returns the world state for the corresponding block number * @@ -889,4 +921,50 @@ private List formatTransactions( private boolean outsideBlockchainRange(final long blockNumber) { return blockNumber > headBlockNumber() || blockNumber < BlockHeader.GENESIS_BLOCK_NUMBER; } + + private Boolean getRemoved(final Hash blockHash, final Supplier isQueryAlive) + throws Exception { + return BackendQuery.runIfAlive( + "matchingLogs - blockIsOnCanonicalChain", + () -> !blockchain.blockIsOnCanonicalChain(blockHash), + isQueryAlive); + } + + private List getTransactions( + final Hash blockHash, final Supplier isQueryAlive) throws Exception { + return BackendQuery.runIfAlive( + "matchingLogs - getBlockBody", + () -> blockchain.getBlockBody(blockHash).orElseThrow().getTransactions(), + isQueryAlive); + } + + private List getReceipts( + final Hash blockHash, final Supplier isQueryAlive) throws Exception { + return BackendQuery.runIfAlive( + "matchingLogs - getTxReceipts", + () -> blockchain.getTxReceipts(blockHash).orElseThrow(), + isQueryAlive); + } + + private Optional getBlockHeader( + final Hash blockHash, final Supplier isQueryAlive) throws Exception { + return BackendQuery.runIfAlive( + "matchingLogs - getBlockHeader", () -> blockchain.getBlockHeader(blockHash), isQueryAlive); + } + + private int logIndexOffset( + final Hash transactionHash, + final List receipts, + final List transactions) { + int logIndexOffset = 0; + for (int i = 0; i < receipts.size(); i++) { + if (transactions.get(i).getHash().equals(transactionHash)) { + break; + } + + logIndexOffset += receipts.get(i).getLogs().size(); + } + + return logIndexOffset; + } } diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/PrivacyQueries.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/PrivacyQueries.java index 7a7bc4919bd..604bad0c9bc 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/PrivacyQueries.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/PrivacyQueries.java @@ -26,6 +26,7 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import java.util.stream.IntStream; import java.util.stream.LongStream; @@ -83,16 +84,24 @@ public List matchingLogs( final long blockNumber = blockHeader.get().getNumber(); final boolean removed = !blockchainQueries.blockIsOnCanonicalChain(blockHash); + final AtomicInteger logIndexOffset = new AtomicInteger(); return IntStream.range(0, privateTransactionReceiptList.size()) .mapToObj( - i -> - LogWithMetadata.generate( - privateTransactionReceiptList.get(i), - blockNumber, - blockHash, - privateTransactionMetadataList.get(i).getPrivateMarkerTransactionHash(), - findPMTIndex(pmtHashList.get(i)), - removed)) + i -> { + final List result = + LogWithMetadata.generate( + logIndexOffset.intValue(), + privateTransactionReceiptList.get(i), + blockNumber, + blockHash, + privateTransactionMetadataList.get(i).getPrivateMarkerTransactionHash(), + findPMTIndex(pmtHashList.get(i)), + removed); + + logIndexOffset.addAndGet(privateTransactionReceiptList.get(i).getLogs().size()); + + return result; + }) .flatMap(Collection::stream) .filter(query::matches) .collect(Collectors.toList()); diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/filter/FilterManagerLogFilterTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/filter/FilterManagerLogFilterTest.java index 8afd5a5bdd3..709f349bf45 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/filter/FilterManagerLogFilterTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/filter/FilterManagerLogFilterTest.java @@ -112,7 +112,7 @@ public void shouldUseBlockHashWhenCheckingLogsForChangesForPrivateFiltersOnly() final Hash blockAddedHash = recordBlockEvents(1).get(0).getBlock().getHash(); - verify(blockchainQueries, never()).matchingLogs(any(), any(), any()); + verify(blockchainQueries, never()).matchingLogs(any(Hash.class), any(LogsQuery.class), any()); verify(privacyQueries).matchingLogs(eq(PRIVACY_GROUP_ID), eq(blockAddedHash), eq(logsQuery())); } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/subscription/logs/LogsSubscriptionServiceTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/subscription/logs/LogsSubscriptionServiceTest.java index aaa5a4a1068..d940a5f8ce5 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/subscription/logs/LogsSubscriptionServiceTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/websocket/subscription/logs/LogsSubscriptionServiceTest.java @@ -90,8 +90,9 @@ public void singleMatchingLogEvent() { final List receipts = blockWithReceipts.getReceipts(); final int txIndex = 1; - final int logIndex = 1; - final Log targetLog = receipts.get(txIndex).getLogsList().get(logIndex); + final int logIndexInTransaction = 1; + final int logIndexInBlock = 3; // third log in the block + final Log targetLog = receipts.get(txIndex).getLogsList().get(logIndexInTransaction); final LogsSubscription subscription = createSubscription(targetLog.getLogger()); registerSubscriptions(subscription); @@ -104,7 +105,8 @@ public void singleMatchingLogEvent() { assertThat(logResults).hasSize(1); final LogResult result = logResults.get(0); - assertLogResultMatches(result, block, receipts, txIndex, logIndex, false); + assertLogResultMatches( + result, block, receipts, txIndex, logIndexInTransaction, logIndexInBlock, false); } @Test @@ -115,8 +117,8 @@ public void singleMatchingLogEmittedThenRemovedInReorg() { final List receipts = blockWithReceipts.getReceipts(); final int txIndex = 1; - final int logIndex = 1; - final Log targetLog = receipts.get(txIndex).getLogsList().get(logIndex); + final int logListIndex = 1; + final Log targetLog = receipts.get(txIndex).getLogsList().get(logListIndex); final LogsSubscription subscription = createSubscription(targetLog.getLogger()); registerSubscriptions(subscription); @@ -138,9 +140,11 @@ public void singleMatchingLogEmittedThenRemovedInReorg() { assertThat(logResults).hasSize(2); final LogResult firstLog = logResults.get(0); - assertLogResultMatches(firstLog, block, receipts, txIndex, logIndex, false); + // third log in the block + assertLogResultMatches(firstLog, block, receipts, txIndex, logListIndex, 3, false); final LogResult secondLog = logResults.get(1); - assertLogResultMatches(secondLog, block, receipts, txIndex, logIndex, true); + // third log in the block, but was removed + assertLogResultMatches(secondLog, block, receipts, txIndex, logListIndex, 3, true); } @Test @@ -151,8 +155,8 @@ public void singleMatchingLogEmittedThenMovedInReorg() { final List receipts = blockWithReceipts.getReceipts(); final int txIndex = 1; - final int logIndex = 1; - final Log targetLog = receipts.get(txIndex).getLogsList().get(logIndex); + final int logListIndex = 1; + final Log targetLog = receipts.get(txIndex).getLogsList().get(logListIndex); final LogsSubscription subscription = createSubscription(targetLog.getLogger()); registerSubscriptions(subscription); @@ -181,12 +185,12 @@ public void singleMatchingLogEmittedThenMovedInReorg() { assertThat(logResults).hasSize(3); final LogResult originalLog = logResults.get(0); - assertLogResultMatches(originalLog, block, receipts, txIndex, logIndex, false); + assertLogResultMatches(originalLog, block, receipts, txIndex, logListIndex, 3, false); final LogResult removedLog = logResults.get(1); - assertLogResultMatches(removedLog, block, receipts, txIndex, logIndex, true); + assertLogResultMatches(removedLog, block, receipts, txIndex, logListIndex, 3, true); final LogResult updatedLog = logResults.get(2); assertLogResultMatches( - updatedLog, newBlockWithLog.getBlock(), newBlockWithLog.getReceipts(), 0, 0, false); + updatedLog, newBlockWithLog.getBlock(), newBlockWithLog.getReceipts(), 0, 0, 0, false); } @Test @@ -218,6 +222,9 @@ public void multipleMatchingLogsEmitted() { // Verify all logs are emitted assertThat(logResults).hasSize(targetBlocks.size() * txCount); + + final List expectedLogIndex = Arrays.asList(0, 2); + for (int i = 0; i < targetBlocks.size(); i++) { final BlockWithReceipts targetBlock = targetBlocks.get(i); for (int j = 0; j < txCount; j++) { @@ -228,6 +235,7 @@ public void multipleMatchingLogsEmitted() { targetBlock.getReceipts(), j, 0, + expectedLogIndex.get(j), false); } } @@ -259,7 +267,7 @@ public void multipleSubscriptionsForSingleMatchingLog() { assertThat(logResults).hasSize(1); final LogResult result = logResults.get(0); - assertLogResultMatches(result, block, receipts, txIndex, logIndex, false); + assertLogResultMatches(result, block, receipts, txIndex, logIndex, 3, false); } } @@ -332,10 +340,11 @@ private void assertLogResultMatches( final Block block, final List receipts, final int txIndex, + final int logListIndex, final int logIndex, final boolean isRemoved) { final Transaction expectedTransaction = block.getBody().getTransactions().get(txIndex); - final Log expectedLog = receipts.get(txIndex).getLogsList().get(logIndex); + final Log expectedLog = receipts.get(txIndex).getLogsList().get(logListIndex); assertThat(result.getLogIndex()).isEqualTo(Quantity.create(logIndex)); assertThat(result.getTransactionIndex()).isEqualTo(Quantity.create(txIndex)); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/LogWithMetadata.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/LogWithMetadata.java index 0eb1485613b..8217216ebb5 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/LogWithMetadata.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/LogWithMetadata.java @@ -59,6 +59,7 @@ public LogWithMetadata( } public static List generate( + final int logIndexOffset, final TransactionReceipt receipt, final long number, final Hash blockHash, @@ -66,26 +67,37 @@ public static List generate( final int transactionIndex, final boolean removed) { return generate( - receipt.getLogsList(), number, blockHash, transactionHash, transactionIndex, removed); + logIndexOffset, + receipt.getLogsList(), + number, + blockHash, + transactionHash, + transactionIndex, + removed); } public static List generate( final Block block, final List receipts, final boolean removed) { final List logsWithMetadata = new ArrayList<>(); + int logIndexOffset = 0; for (int txi = 0; txi < receipts.size(); ++txi) { - logsWithMetadata.addAll( + final List logs = generate( + logIndexOffset, receipts.get(txi), block.getHeader().getNumber(), block.getHash(), block.getBody().getTransactions().get(txi).getHash(), txi, - removed)); + removed); + logIndexOffset += logs.size(); + logsWithMetadata.addAll(logs); } return logsWithMetadata; } public static List generate( + final int logIndexOffset, final PrivateTransactionReceipt receipt, final long number, final Hash blockHash, @@ -93,10 +105,17 @@ public static List generate( final int transactionIndex, final boolean removed) { return generate( - receipt.getLogs(), number, blockHash, transactionHash, transactionIndex, removed); + logIndexOffset, + receipt.getLogs(), + number, + blockHash, + transactionHash, + transactionIndex, + removed); } private static List generate( + final int logIndexOffset, final List receiptLogs, final long number, final Hash blockHash, @@ -108,7 +127,7 @@ private static List generate( for (int logIndex = 0; logIndex < receiptLogs.size(); ++logIndex) { logs.add( new LogWithMetadata( - logIndex, + logIndexOffset + logIndex, number, blockHash, transactionHash,