Skip to content

Commit

Permalink
Fix and test that the BlockAwareOperationTracer methods are invoked t…
Browse files Browse the repository at this point in the history 
…he correct number of times (hyperledger#6259)

* Test that the BlockAwareOperationTracer are invoked the correct number of times
* Remove redundant calls to traceEndBlock

Signed-off-by: Fabio Di Fabio <[email protected]>
  • Loading branch information
@fab-10
fab-10 authored Dec 7, 2023
1 parent ecb23ba commit 3bfe597
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 11 deletions.
2 changes: 0 additions & 2 deletions besu/src/main/java/org/hyperledger/besu/services/TraceServiceImpl.java
View file
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
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);
}
}
}
}

0 comments on commit 3bfe597

Please sign in to comment.