Skip to content

Fix and test that the BlockAwareOperationTracer methods are invoked the correct number of times #6259

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
garyschulte merged 2 commits into from
Dec 7, 2023
Merged

Fix and test that the BlockAwareOperationTracer methods are invoked the correct number of times #6259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af77b4f
Test that the BlockAwareOperationTracer are invoked the correct numbe…
fab-10 Dec 6, 2023
6f0cb33
Remove redundant calls to traceEndBlock
fab-10 Dec 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions besu/src/main/java/org/hyperledger/besu/services/TraceServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ public void trace(
blocks.forEach(
block -> {
results.addAll(trace(blockchain, block, chainUpdater, tracer));
tracer.traceEndBlock(block.getHeader(), block.getBody());
});
afterTracing.accept(chainUpdater.getNextUpdater());
return Optional.of(results);
Expand All @@ -180,7 +179,6 @@ private Optional<List<TransactionProcessingResult>> trace(
block.getHash(),
traceableState ->
Optional.of(trace(blockchain, block, new ChainUpdater(traceableState), tracer)));
tracer.traceEndBlock(block.getHeader(), block.getBody());

return results;
}
Expand Down
103 changes: 94 additions & 9 deletions besu/src/test/java/org/hyperledger/besu/services/TraceServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,31 +15,46 @@
package org.hyperledger.besu.services;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;

import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.datatypes.Transaction;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.api.query.BlockchainQueries;
import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
import org.hyperledger.besu.ethereum.core.Block;
import org.hyperledger.besu.ethereum.core.BlockchainSetupUtil;
import org.hyperledger.besu.ethereum.worldstate.DataStorageFormat;
import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive;
import org.hyperledger.besu.evm.log.Log;
import org.hyperledger.besu.evm.worldstate.WorldView;
import org.hyperledger.besu.plugin.data.BlockBody;
import org.hyperledger.besu.plugin.data.BlockHeader;
import org.hyperledger.besu.plugin.data.BlockTraceResult;
import org.hyperledger.besu.plugin.data.TransactionTraceResult;
import org.hyperledger.besu.plugin.services.TraceService;
import org.hyperledger.besu.plugin.services.tracer.BlockAwareOperationTracer;

import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.LongStream;

import org.apache.tuweni.bytes.Bytes;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.runner.RunWith;
import org.mockito.junit.MockitoJUnitRunner;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.junit.jupiter.MockitoExtension;

@RunWith(MockitoJUnitRunner.class)
@ExtendWith(MockitoExtension.class)
class TraceServiceImplTest {

TraceService traceService;
Expand Down Expand Up @@ -73,19 +88,40 @@ void shouldRetrieveStateUpdatePostTracingForOneBlock() {
final long persistedNonceForAccount =
worldStateArchive.getMutable().get(addressToVerify).getNonce();

final long blockNumber = 2;

final BlockAwareOperationTracer opTracer = mock(BlockAwareOperationTracer.class);

traceService.trace(
2,
2,
blockNumber,
blockNumber,
worldState -> {
assertThat(worldState.get(addressToVerify).getNonce()).isEqualTo(1);
},
worldState -> {
assertThat(worldState.get(addressToVerify).getNonce()).isEqualTo(2);
},
BlockAwareOperationTracer.NO_TRACING);
opTracer);

assertThat(worldStateArchive.getMutable().get(addressToVerify).getNonce())
.isEqualTo(persistedNonceForAccount);

final Block tracedBlock = blockchain.getBlockByNumber(blockNumber).get();

verify(opTracer).traceStartBlock(tracedBlock.getHeader(), tracedBlock.getBody());

tracedBlock
.getBody()
.getTransactions()
.forEach(
tx -> {
verify(opTracer).traceStartTransaction(any(), eq(tx));
verify(opTracer)
.traceEndTransaction(
any(), eq(tx), anyBoolean(), any(), any(), anyLong(), anyLong());
});

verify(opTracer).traceEndBlock(tracedBlock.getHeader(), tracedBlock.getBody());
}

@Test
Expand All @@ -96,20 +132,44 @@ void shouldRetrieveStateUpdatePostTracingForAllBlocks() {
final long persistedNonceForAccount =
worldStateArchive.getMutable().get(addressToVerify).getNonce();

final long startBlock = 1;
final long endBlock = 32;
final BlockAwareOperationTracer opTracer = mock(BlockAwareOperationTracer.class);

traceService.trace(
0,
32,
startBlock,
endBlock,
worldState -> {
assertThat(worldState.get(addressToVerify).getNonce()).isEqualTo(0);
},
worldState -> {
assertThat(worldState.get(addressToVerify).getNonce())
.isEqualTo(persistedNonceForAccount);
},
BlockAwareOperationTracer.NO_TRACING);
opTracer);

assertThat(worldStateArchive.getMutable().get(addressToVerify).getNonce())
.isEqualTo(persistedNonceForAccount);

LongStream.rangeClosed(startBlock, endBlock)
.mapToObj(blockchain::getBlockByNumber)
.map(Optional::get)
.forEach(
tracedBlock -> {
verify(opTracer).traceStartBlock(tracedBlock.getHeader(), tracedBlock.getBody());
tracedBlock
.getBody()
.getTransactions()
.forEach(
tx -> {
verify(opTracer).traceStartTransaction(any(), eq(tx));
verify(opTracer)
.traceEndTransaction(
any(), eq(tx), anyBoolean(), any(), any(), anyLong(), anyLong());
});

verify(opTracer).traceEndBlock(tracedBlock.getHeader(), tracedBlock.getBody());
});
}

@Test
Expand Down Expand Up @@ -197,8 +257,16 @@ private static class TxStartEndTracer implements BlockAwareOperationTracer {
public long txEndGasUsed;
public Long txEndTimeNs;

private final Set<Transaction> traceStartTxCalled = new HashSet<>();
private final Set<Transaction> traceEndTxCalled = new HashSet<>();
private final Set<Hash> traceStartBlockCalled = new HashSet<>();
private final Set<Hash> traceEndBlockCalled = new HashSet<>();

@Override
public void traceStartTransaction(final WorldView worldView, final Transaction transaction) {
if (!traceStartTxCalled.add(transaction)) {
fail("traceStartTransaction already called for tx " + transaction);
}
txStartWorldView = worldView;
txStartTransaction = transaction;
}
Expand All @@ -212,6 +280,9 @@ public void traceEndTransaction(
final List<Log> logs,
final long gasUsed,
final long timeNs) {
if (!traceEndTxCalled.add(transaction)) {
fail("traceEndTransaction already called for tx " + transaction);
}
txEndWorldView = worldView;
txEndTransaction = transaction;
txEndStatus = status;
Expand All @@ -220,5 +291,19 @@ public void traceEndTransaction(
txEndGasUsed = gasUsed;
txEndTimeNs = timeNs;
}

@Override
public void traceStartBlock(final BlockHeader blockHeader, final BlockBody blockBody) {
if (!traceStartBlockCalled.add(blockHeader.getBlockHash())) {
fail("traceStartBlock already called for block " + blockHeader);
}
}

@Override
public void traceEndBlock(final BlockHeader blockHeader, final BlockBody blockBody) {
if (!traceEndBlockCalled.add(blockHeader.getBlockHash())) {
fail("traceEndBlock already called for block " + blockHeader);
}
}
}
}