diff --git a/CHANGELOG.md b/CHANGELOG.md index ed1824aa480..5ae40e29734 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,14 +2,16 @@ ## 22.7.5 - ### Additions and Improvements +- Avoid sending added block events to transaction pool, and processing incoming transactions during initial sync [#4457](https://github.com/hyperledger/besu/pull/4457) +- When building a new proposal, keep the best block built until now instead of the last one [#4455](https://github.com/hyperledger/besu/pull/4455) +- Add Mainnet to merged networks [#4463](https://github.com/hyperledger/besu/pull/4463) + ### Bug Fixes ### Download Links ## 22.7.4 - ### Bug Fixes - Remove records that track transactions by sender when they are empty to same memory in the transaction pool [#4415](https://github.com/hyperledger/besu/pull/4415) - Add Toml configuration file support for _--Xplugin-rocksdb-high-spec-enabled_ flag [#4438](https://github.com/hyperledger/besu/pull/4438) diff --git a/SECURITY.md b/SECURITY.md index 4727d240345..9a328164b95 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -7,13 +7,15 @@ hear from you. We will take all security bugs seriously and if confirmed upon in patch it within a reasonable amount of time and release a public security bulletin discussing the impact and credit the discoverer. -There are two ways to report a security bug. The easiest is to email a description of the flaw and -any related information (e.g. reproduction steps, version) to -[security at hyperledger dot org](mailto:security@hyperledger.org). - -The other way is to file a confidential security bug in our -[JIRA bug tracking system](https://jira.hyperledger.org). Be sure to set the “Security Level” to -“Security issue”. +There are two email addresses where Hyperledger Besu accepts security bugs. The +first, [security "dash" besu at lists dot hyperledger dot org](mailto:security-besu@lists.hyperledger.org) +is limited to a subset of Hyperledger Besu maintainers and Hyperledger staff. For highly sensitive +bugs this is a preferred address. The second email +address [security at hyperledger dot org](mailto:security@hyperledger.org) is limited to a subset of +maintainers and staff of all Hyperledger projects, and may be viewed by maintainers outside of +Hyperledger Besu. When sending information to either of these emails please be sure to include a +description of the flaw and any related information (e.g. reproduction steps, version, known active +use). The process by which the Hyperledger Security Team handles security bugs is documented further in our [Defect Response page](https://wiki.hyperledger.org/display/SEC/Defect+Response) on our diff --git a/acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/01_prepare_payload.json b/acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/01_prepare_payload.json index 543e12d3c7d..3ab6f968fbc 100644 --- a/acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/01_prepare_payload.json +++ b/acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/01_prepare_payload.json @@ -25,7 +25,7 @@ "latestValidHash": "0x3b8fb240d288781d4aac94d3fd16809ee413bc99294a085798a589dae51ddd4a", "validationError": null }, - "payloadId": "0x0000000021f32cc1" + "payloadId": "0x0065bd195a9b3bfb" } }, "statusCode" : 200 diff --git a/acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/02_get_payload.json b/acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/02_get_payload.json index dbb97e518dd..e5257090527 100644 --- a/acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/02_get_payload.json +++ b/acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/02_get_payload.json @@ -3,7 +3,7 @@ "jsonrpc": "2.0", "method": "engine_getPayloadV1", "params": [ - "0x0000000021f32cc1" + "0x0065bd195a9b3bfb" ], "id": 67 }, diff --git a/besu/src/main/java/org/hyperledger/besu/cli/config/NetworkName.java b/besu/src/main/java/org/hyperledger/besu/cli/config/NetworkName.java index 5505a0853fe..1cf24da8481 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/config/NetworkName.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/config/NetworkName.java @@ -89,6 +89,7 @@ public Optional getDeprecationDate() { public static boolean isMergedNetwork(final NetworkName networkName) { switch (networkName) { + case MAINNET: case GOERLI: case ROPSTEN: case SEPOLIA: diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index f804ad23deb..ad97be510ad 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -386,7 +386,7 @@ public BesuController build() { ethContext, clock, metricsSystem, - syncState::isInitialSyncPhaseDone, + syncState, miningParameters, transactionPoolConfiguration); diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java index 40095144919..367345ab232 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -882,7 +882,7 @@ public void noOverrideDefaultValuesIfKeyIsNotPresentInConfigFile() throws IOExce final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue(); assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.FAST); - assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(5); + assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(1); assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); @@ -1709,19 +1709,19 @@ public void helpShouldDisplayFastSyncOptions() { } @Test - public void checkValidDefaultFastSyncMinPeersOption() { - parseCommand("--sync-mode", "FAST"); + public void checkValidDefaultFastSyncMinPeersPoS() { + parseCommand("--sync-mode", "FAST", "--network", "MAINNET"); verify(mockControllerBuilder).synchronizerConfiguration(syncConfigurationCaptor.capture()); final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue(); assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.FAST); - assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(5); + assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(1); assertThat(commandOutput.toString(UTF_8)).isEmpty(); assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); } @Test - public void checkValidDefaultFastSyncMinPeersPreMergeOption() { + public void checkValidDefaultFastSyncMinPeersPoW() { parseCommand("--sync-mode", "FAST", "--network", "CLASSIC"); verify(mockControllerBuilder).synchronizerConfiguration(syncConfigurationCaptor.capture()); @@ -1732,18 +1732,6 @@ public void checkValidDefaultFastSyncMinPeersPreMergeOption() { assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); } - @Test - public void checkValidDefaultFastSyncMinPeersPostMergeOption() { - parseCommand("--sync-mode", "FAST", "--network", "GOERLI"); - verify(mockControllerBuilder).synchronizerConfiguration(syncConfigurationCaptor.capture()); - - final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue(); - assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.FAST); - assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(1); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); - } - @Test public void parsesValidFastSyncMinPeersOption() { parseCommand("--sync-mode", "FAST", "--fast-sync-min-peers", "11"); diff --git a/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java b/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java index ab08f2010ed..21d837ca3bb 100644 --- a/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java +++ b/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java @@ -147,7 +147,7 @@ public void setUp() { mockEthContext, TestClock.system(ZoneId.systemDefault()), new NoOpMetricsSystem(), - syncState::isInitialSyncPhaseDone, + syncState, new MiningParameters.Builder().minTransactionGasPrice(Wei.ZERO).build(), txPoolConfig); diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java index 6923419ff8f..406229f2ecf 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java @@ -14,6 +14,8 @@ */ package org.hyperledger.besu.consensus.merge; +import static org.hyperledger.besu.util.Slf4jLambdaHelper.debugLambda; + import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier; import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.ConsensusContext; @@ -23,6 +25,7 @@ import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.util.Subscribers; +import java.util.Comparator; import java.util.Optional; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; @@ -30,12 +33,18 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.EvictingQueue; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class PostMergeContext implements MergeContext { + private static final Logger LOG = LoggerFactory.getLogger(PostMergeContext.class); static final int MAX_BLOCKS_IN_PROGRESS = 12; private static final AtomicReference singleton = new AtomicReference<>(); + private static final Comparator compareByGasUsedDesc = + Comparator.comparingLong((Block block) -> block.getHeader().getGasUsed()).reversed(); + private final AtomicReference syncState; private final AtomicReference terminalTotalDifficulty; // initial postMerge state is indeterminate until it is set: @@ -197,21 +206,57 @@ public boolean validateCandidateHead(final BlockHeader candidateHeader) { } @Override - public void putPayloadById(final PayloadIdentifier payloadId, final Block block) { - var priorsById = retrieveTuplesById(payloadId).collect(Collectors.toUnmodifiableList()); - blocksInProgress.add(new PayloadTuple(payloadId, block)); - priorsById.stream().forEach(blocksInProgress::remove); + public void putPayloadById(final PayloadIdentifier payloadId, final Block newBlock) { + synchronized (blocksInProgress) { + final Optional maybeCurrBestBlock = retrieveBlockById(payloadId); + + maybeCurrBestBlock.ifPresentOrElse( + currBestBlock -> { + if (compareByGasUsedDesc.compare(newBlock, currBestBlock) < 0) { + debugLambda( + LOG, + "New proposal for payloadId {} {} is better than the previous one {}", + payloadId::toHexString, + () -> logBlockProposal(newBlock), + () -> logBlockProposal(currBestBlock)); + blocksInProgress.removeAll( + retrieveTuplesById(payloadId).collect(Collectors.toUnmodifiableList())); + blocksInProgress.add(new PayloadTuple(payloadId, newBlock)); + } + }, + () -> blocksInProgress.add(new PayloadTuple(payloadId, newBlock))); + + debugLambda( + LOG, + "Current best proposal for payloadId {} {}", + payloadId::toHexString, + () -> retrieveBlockById(payloadId).map(bb -> logBlockProposal(bb)).orElse("N/A")); + } } @Override public Optional retrieveBlockById(final PayloadIdentifier payloadId) { - return retrieveTuplesById(payloadId).map(tuple -> tuple.block).findFirst(); + synchronized (blocksInProgress) { + return retrieveTuplesById(payloadId) + .map(tuple -> tuple.block) + .sorted(compareByGasUsedDesc) + .findFirst(); + } } private Stream retrieveTuplesById(final PayloadIdentifier payloadId) { return blocksInProgress.stream().filter(z -> z.payloadIdentifier.equals(payloadId)); } + private String logBlockProposal(final Block block) { + return "block " + + block.toLogString() + + " gas used " + + block.getHeader().getGasUsed() + + " transactions " + + block.getBody().getTransactions().size(); + } + private static class PayloadTuple { final PayloadIdentifier payloadIdentifier; final Block block; diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java index 3e75367cb87..315bbeca97d 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java @@ -175,17 +175,18 @@ public void changeTargetGasLimit(final Long newTargetGasLimit) { public PayloadIdentifier preparePayload( final BlockHeader parentHeader, final Long timestamp, - final Bytes32 random, + final Bytes32 prevRandao, final Address feeRecipient) { final PayloadIdentifier payloadIdentifier = - PayloadIdentifier.forPayloadParams(parentHeader.getBlockHash(), timestamp); + PayloadIdentifier.forPayloadParams( + parentHeader.getBlockHash(), timestamp, prevRandao, feeRecipient); final MergeBlockCreator mergeBlockCreator = this.mergeBlockCreator.forParams(parentHeader, Optional.ofNullable(feeRecipient)); // put the empty block in first final Block emptyBlock = - mergeBlockCreator.createBlock(Optional.of(Collections.emptyList()), random, timestamp); + mergeBlockCreator.createBlock(Optional.of(Collections.emptyList()), prevRandao, timestamp); Result result = validateBlock(emptyBlock); if (result.blockProcessingOutputs.isPresent()) { @@ -202,7 +203,7 @@ public PayloadIdentifier preparePayload( result.errorMessage); } - tryToBuildBetterBlock(timestamp, random, payloadIdentifier, mergeBlockCreator); + tryToBuildBetterBlock(timestamp, prevRandao, payloadIdentifier, mergeBlockCreator); return payloadIdentifier; } diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeMiningCoordinator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeMiningCoordinator.java index d83d16eb2c3..a09a2ac4eaa 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeMiningCoordinator.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeMiningCoordinator.java @@ -33,7 +33,7 @@ public interface MergeMiningCoordinator extends MiningCoordinator { PayloadIdentifier preparePayload( final BlockHeader parentHeader, final Long timestamp, - final Bytes32 random, + final Bytes32 prevRandao, final Address feeRecipient); @Override diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadIdentifier.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadIdentifier.java index db647704644..a8568fa560e 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadIdentifier.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadIdentifier.java @@ -14,6 +14,7 @@ */ package org.hyperledger.besu.consensus.merge.blockcreation; +import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.plugin.data.Quantity; @@ -21,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonValue; +import org.apache.tuweni.bytes.Bytes32; import org.apache.tuweni.units.bigints.UInt64; public class PayloadIdentifier implements Quantity { @@ -36,8 +38,16 @@ public PayloadIdentifier(final Long payloadId) { this.val = UInt64.valueOf(Math.abs(payloadId)); } - public static PayloadIdentifier forPayloadParams(final Hash parentHash, final Long timestamp) { - return new PayloadIdentifier(((long) parentHash.toHexString().hashCode()) ^ timestamp); + public static PayloadIdentifier forPayloadParams( + final Hash parentHash, + final Long timestamp, + final Bytes32 prevRandao, + final Address feeRecipient) { + return new PayloadIdentifier( + timestamp + ^ ((long) parentHash.toHexString().hashCode()) << 8 + ^ ((long) prevRandao.toHexString().hashCode()) << 16 + ^ ((long) feeRecipient.toHexString().hashCode()) << 24); } @Override diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/TransitionCoordinator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/TransitionCoordinator.java index e8d968e17ea..4bd465e81da 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/TransitionCoordinator.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/TransitionCoordinator.java @@ -127,9 +127,9 @@ public void changeTargetGasLimit(final Long targetGasLimit) { public PayloadIdentifier preparePayload( final BlockHeader parentHeader, final Long timestamp, - final Bytes32 random, + final Bytes32 prevRandao, final Address feeRecipient) { - return mergeCoordinator.preparePayload(parentHeader, timestamp, random, feeRecipient); + return mergeCoordinator.preparePayload(parentHeader, timestamp, prevRandao, feeRecipient); } @Override diff --git a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java index 5f022df5f0e..fe9af337ecd 100644 --- a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java +++ b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java @@ -131,6 +131,7 @@ public void candidateHeadIsInvalidIfBeforeFinalizedBlock() { @Test public void putAndRetrieveFirstPayload() { Block mockBlock = mock(Block.class); + PayloadIdentifier firstPayloadId = new PayloadIdentifier(1L); postMergeContext.putPayloadById(firstPayloadId, mockBlock); @@ -138,14 +139,47 @@ public void putAndRetrieveFirstPayload() { } @Test - public void puttingTwoBlocksWithTheSamePayloadIdWeRetrieveTheLatter() { - Block mockBlock1 = mock(Block.class); - Block mockBlock2 = mock(Block.class); + public void puttingTwoBlocksWithTheSamePayloadIdWeRetrieveTheBest() { + BlockHeader zeroTxBlockHeader = mock(BlockHeader.class); + when(zeroTxBlockHeader.getGasUsed()).thenReturn(0L); + Block zeroTxBlock = mock(Block.class); + when(zeroTxBlock.getHeader()).thenReturn(zeroTxBlockHeader); + + BlockHeader betterBlockHeader = mock(BlockHeader.class); + when(betterBlockHeader.getGasUsed()).thenReturn(11L); + Block betterBlock = mock(Block.class); + when(betterBlock.getHeader()).thenReturn(betterBlockHeader); + + PayloadIdentifier payloadId = new PayloadIdentifier(1L); + postMergeContext.putPayloadById(payloadId, zeroTxBlock); + postMergeContext.putPayloadById(payloadId, betterBlock); + + assertThat(postMergeContext.retrieveBlockById(payloadId)).contains(betterBlock); + } + + @Test + public void puttingABlockWithTheSamePayloadIdSmallerThanAnExistingOneWeRetrieveTheBest() { + BlockHeader zeroTxBlockHeader = mock(BlockHeader.class); + when(zeroTxBlockHeader.getGasUsed()).thenReturn(0L); + Block zeroTxBlock = mock(Block.class); + when(zeroTxBlock.getHeader()).thenReturn(zeroTxBlockHeader); + + BlockHeader betterBlockHeader = mock(BlockHeader.class); + when(betterBlockHeader.getGasUsed()).thenReturn(11L); + Block betterBlock = mock(Block.class); + when(betterBlock.getHeader()).thenReturn(betterBlockHeader); + + BlockHeader smallBlockHeader = mock(BlockHeader.class); + when(smallBlockHeader.getGasUsed()).thenReturn(5L); + Block smallBlock = mock(Block.class); + when(smallBlock.getHeader()).thenReturn(smallBlockHeader); + PayloadIdentifier payloadId = new PayloadIdentifier(1L); - postMergeContext.putPayloadById(payloadId, mockBlock1); - postMergeContext.putPayloadById(payloadId, mockBlock2); + postMergeContext.putPayloadById(payloadId, zeroTxBlock); + postMergeContext.putPayloadById(payloadId, betterBlock); + postMergeContext.putPayloadById(payloadId, smallBlock); - assertThat(postMergeContext.retrieveBlockById(payloadId)).contains(mockBlock2); + assertThat(postMergeContext.retrieveBlockById(payloadId)).contains(betterBlock); } @Test diff --git a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadIdentifierTest.java b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadIdentifierTest.java index 9203db85d4d..1c19ead4e33 100644 --- a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadIdentifierTest.java +++ b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadIdentifierTest.java @@ -16,10 +16,12 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Hash; import java.util.List; +import org.apache.tuweni.bytes.Bytes32; import org.junit.Test; public class PayloadIdentifierTest { @@ -39,7 +41,9 @@ public void serializesToEvenHexRepresentation() { @Test public void conversionCoverage() { - var idTest = PayloadIdentifier.forPayloadParams(Hash.ZERO, 1337L); + var idTest = + PayloadIdentifier.forPayloadParams( + Hash.ZERO, 1337L, Bytes32.random(), Address.fromHexString("0x42")); assertThat(new PayloadIdentifier(idTest.getAsBigInteger().longValue())).isEqualTo(idTest); assertThat(new PayloadIdentifier(idTest.getAsBigInteger().longValue())).isEqualTo(idTest); } diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdated.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdated.java index 244d4773799..1796ca63db1 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdated.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdated.java @@ -72,6 +72,8 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) final Optional maybePayloadAttributes = requestContext.getOptionalParameter(1, EnginePayloadAttributesParameter.class); + LOG.debug("Forkchoice parameters {}", forkChoice); + Optional maybeFinalizedHash = Optional.ofNullable(forkChoice.getFinalizedBlockHash()) .filter(finalized -> !finalized.isZero()); diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/EngineForkchoiceUpdatedParameter.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/EngineForkchoiceUpdatedParameter.java index b6d70420fe5..7b3e50a07e1 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/EngineForkchoiceUpdatedParameter.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/EngineForkchoiceUpdatedParameter.java @@ -47,4 +47,14 @@ public EngineForkchoiceUpdatedParameter( this.headBlockHash = headBlockHash; this.safeBlockHash = safeBlockHash; } + + @Override + public String toString() { + return "headBlockHash=" + + headBlockHash + + ", safeBlockHash=" + + safeBlockHash + + ", finalizedBlockHash=" + + finalizedBlockHash; + } } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedTest.java index e19fa6de2b7..f2351d48853 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedTest.java @@ -227,7 +227,11 @@ public void shouldReturnValidWithoutFinalizedWithPayload() { Bytes32.fromHexStringLenient("0xDEADBEEF").toHexString(), Address.ECREC.toString()); var mockPayloadId = - PayloadIdentifier.forPayloadParams(mockHeader.getHash(), payloadParams.getTimestamp()); + PayloadIdentifier.forPayloadParams( + mockHeader.getHash(), + payloadParams.getTimestamp(), + payloadParams.getPrevRandao(), + payloadParams.getSuggestedFeeRecipient()); when(mergeCoordinator.preparePayload( mockHeader, payloadParams.getTimestamp(), payloadParams.getPrevRandao(), Address.ECREC)) diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadTest.java index 4695bbf4c7e..0d82d196e3f 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadTest.java @@ -21,6 +21,7 @@ import org.hyperledger.besu.consensus.merge.MergeContext; import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier; +import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod; @@ -55,7 +56,8 @@ public class EngineGetPayloadTest { private static final Vertx vertx = Vertx.vertx(); private static final BlockResultFactory factory = new BlockResultFactory(); private static final PayloadIdentifier mockPid = - PayloadIdentifier.forPayloadParams(Hash.ZERO, 1337L); + PayloadIdentifier.forPayloadParams( + Hash.ZERO, 1337L, Bytes32.random(), Address.fromHexString("0x42")); private static final BlockHeader mockHeader = new BlockHeaderTestFixture().prevRandao(Bytes32.random()).buildHeader(); private static final Block mockBlock = @@ -99,7 +101,10 @@ public void shouldReturnBlockForKnownPayloadId() { @Test public void shouldFailForUnknownPayloadId() { - var resp = resp(PayloadIdentifier.forPayloadParams(Hash.ZERO, 0L)); + var resp = + resp( + PayloadIdentifier.forPayloadParams( + Hash.ZERO, 0L, Bytes32.random(), Address.fromHexString("0x42"))); assertThat(resp).isInstanceOf(JsonRpcErrorResponse.class); verify(engineCallListener, times(1)).executionEngineCalled(); } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java index 681920ed9a1..c8d6db5f84b 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java @@ -138,13 +138,11 @@ public static ProtocolSpecBuilder frontierDefinition( FeeMarket.legacy(), CoinbaseFeePriceCalculator.frontier())) .privateTransactionProcessorBuilder( - (gasCalculator, - transactionValidator, + (transactionValidator, contractCreationProcessor, messageCallProcessor, privateTransactionValidator) -> new PrivateTransactionProcessor( - gasCalculator, transactionValidator, contractCreationProcessor, messageCallProcessor, @@ -357,13 +355,11 @@ public static ProtocolSpecBuilder byzantiumDefinition( .blockReward(BYZANTIUM_BLOCK_REWARD) .privateTransactionValidatorBuilder(() -> new PrivateTransactionValidator(chainId)) .privateTransactionProcessorBuilder( - (gasCalculator, - transactionValidator, + (transactionValidator, contractCreationProcessor, messageCallProcessor, privateTransactionValidator) -> new PrivateTransactionProcessor( - gasCalculator, transactionValidator, contractCreationProcessor, messageCallProcessor, diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ProtocolSpecBuilder.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ProtocolSpecBuilder.java index 8216998f034..134d9285e9f 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ProtocolSpecBuilder.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ProtocolSpecBuilder.java @@ -315,7 +315,6 @@ public ProtocolSpec build(final ProtocolSchedule protocolSchedule) { privateTransactionValidatorBuilder.apply(); privateTransactionProcessor = privateTransactionProcessorBuilder.apply( - gasCalculator, transactionValidator, contractCreationProcessor, messageCallProcessor, @@ -392,7 +391,6 @@ MainnetTransactionProcessor apply( public interface PrivateTransactionProcessorBuilder { PrivateTransactionProcessor apply( - GasCalculator gasCalculator, MainnetTransactionValidator transactionValidator, AbstractMessageProcessor contractCreationProcessor, AbstractMessageProcessor messageCallProcessor, diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionProcessor.java index 8410310bcae..7ff7dfad38a 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionProcessor.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionProcessor.java @@ -20,7 +20,6 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader; -import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.mainnet.MainnetTransactionValidator; import org.hyperledger.besu.ethereum.mainnet.ValidationResult; import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult; @@ -31,7 +30,6 @@ import org.hyperledger.besu.evm.account.EvmAccount; import org.hyperledger.besu.evm.account.MutableAccount; import org.hyperledger.besu.evm.frame.MessageFrame; -import org.hyperledger.besu.evm.gascalculator.GasCalculator; import org.hyperledger.besu.evm.processor.AbstractMessageProcessor; import org.hyperledger.besu.evm.tracing.OperationTracer; import org.hyperledger.besu.evm.worldstate.WorldUpdater; @@ -50,8 +48,6 @@ public class PrivateTransactionProcessor { private static final Logger LOG = LoggerFactory.getLogger(PrivateTransactionProcessor.class); - private final GasCalculator gasCalculator; - @SuppressWarnings("unused") private final MainnetTransactionValidator transactionValidator; @@ -67,14 +63,12 @@ public class PrivateTransactionProcessor { private final boolean clearEmptyAccounts; public PrivateTransactionProcessor( - final GasCalculator gasCalculator, final MainnetTransactionValidator transactionValidator, final AbstractMessageProcessor contractCreationProcessor, final AbstractMessageProcessor messageCallProcessor, final boolean clearEmptyAccounts, final int maxStackSize, final PrivateTransactionValidator privateTransactionValidator) { - this.gasCalculator = gasCalculator; this.transactionValidator = transactionValidator; this.contractCreationProcessor = contractCreationProcessor; this.messageCallProcessor = messageCallProcessor; @@ -228,14 +222,4 @@ private AbstractMessageProcessor getMessageProcessor(final MessageFrame.Type typ throw new IllegalStateException("Request for unsupported message processor type " + type); } } - - @SuppressWarnings("unused") - private long refunded( - final Transaction transaction, final long gasRemaining, final long gasRefund) { - // Integer truncation takes care of the the floor calculation needed after the divide. - final long maxRefundAllowance = - (transaction.getGasLimit() - gasRemaining) / gasCalculator.getMaxRefundQuotient(); - final long refundAllowance = Math.min(maxRefundAllowance, gasRefund); - return gasRemaining + refundAllowance; - } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessor.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessor.java index e0e00dd4aae..7fc5bd5406b 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessor.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessor.java @@ -33,7 +33,6 @@ import java.time.Instant; import java.util.List; import java.util.concurrent.ConcurrentHashMap; -import java.util.function.Supplier; import java.util.stream.Collectors; import org.slf4j.Logger; @@ -55,21 +54,18 @@ public class NewPooledTransactionHashesMessageProcessor { private final TransactionPoolConfiguration transactionPoolConfiguration; private final EthContext ethContext; private final MetricsSystem metricsSystem; - private final Supplier shouldProcessMessages; public NewPooledTransactionHashesMessageProcessor( final PeerTransactionTracker transactionTracker, final TransactionPool transactionPool, final TransactionPoolConfiguration transactionPoolConfiguration, final EthContext ethContext, - final MetricsSystem metricsSystem, - final Supplier shouldProcessMessages) { + final MetricsSystem metricsSystem) { this.transactionTracker = transactionTracker; this.transactionPool = transactionPool; this.transactionPoolConfiguration = transactionPoolConfiguration; this.ethContext = ethContext; this.metricsSystem = metricsSystem; - this.shouldProcessMessages = shouldProcessMessages; this.totalSkippedNewPooledTransactionHashesMessageCounter = new RunnableCounter( metricsSystem.createCounter( @@ -110,26 +106,24 @@ private void processNewPooledTransactionHashesMessage( incomingTransactionHashes::size, incomingTransactionHashes::toString); - if (shouldProcessMessages.get()) { - final BufferedGetPooledTransactionsFromPeerFetcher bufferedTask = - scheduledTasks.computeIfAbsent( - peer, - ethPeer -> { - ethContext - .getScheduler() - .scheduleFutureTask( - new FetcherCreatorTask(peer), - transactionPoolConfiguration.getEth65TrxAnnouncedBufferingPeriod()); - - return new BufferedGetPooledTransactionsFromPeerFetcher( - ethContext, peer, transactionPool, transactionTracker, metricsSystem); - }); - - bufferedTask.addHashes( - incomingTransactionHashes.stream() - .filter(hash -> transactionPool.getTransactionByHash(hash).isEmpty()) - .collect(Collectors.toList())); - } + final BufferedGetPooledTransactionsFromPeerFetcher bufferedTask = + scheduledTasks.computeIfAbsent( + peer, + ethPeer -> { + ethContext + .getScheduler() + .scheduleFutureTask( + new FetcherCreatorTask(peer), + transactionPoolConfiguration.getEth65TrxAnnouncedBufferingPeriod()); + + return new BufferedGetPooledTransactionsFromPeerFetcher( + ethContext, peer, transactionPool, transactionTracker, metricsSystem); + }); + + bufferedTask.addHashes( + incomingTransactionHashes.stream() + .filter(hash -> transactionPool.getTransactionByHash(hash).isEmpty()) + .collect(Collectors.toList())); } catch (final RLPException ex) { if (peer != null) { LOG.debug( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java index 02a01f85706..b2aea31c7a1 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java @@ -19,6 +19,7 @@ import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.messages.EthPV62; import org.hyperledger.besu.ethereum.eth.messages.EthPV65; +import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.sorter.AbstractPendingTransactionsSorter; import org.hyperledger.besu.ethereum.eth.transactions.sorter.BaseFeePendingTransactionsSorter; import org.hyperledger.besu.ethereum.eth.transactions.sorter.GasPricePendingTransactionsSorter; @@ -28,9 +29,12 @@ import org.hyperledger.besu.plugin.services.MetricsSystem; import java.time.Clock; -import java.util.function.Supplier; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class TransactionPoolFactory { + private static final Logger LOG = LoggerFactory.getLogger(TransactionPoolFactory.class); public static TransactionPool createTransactionPool( final ProtocolSchedule protocolSchedule, @@ -38,7 +42,7 @@ public static TransactionPool createTransactionPool( final EthContext ethContext, final Clock clock, final MetricsSystem metricsSystem, - final Supplier shouldProcessTransactions, + final SyncState syncState, final MiningParameters miningParameters, final TransactionPoolConfiguration transactionPoolConfiguration) { @@ -58,7 +62,7 @@ public static TransactionPool createTransactionPool( protocolContext, ethContext, metricsSystem, - shouldProcessTransactions, + syncState, miningParameters, transactionPoolConfiguration, pendingTransactions, @@ -72,7 +76,7 @@ static TransactionPool createTransactionPool( final ProtocolContext protocolContext, final EthContext ethContext, final MetricsSystem metricsSystem, - final Supplier shouldProcessTransactions, + final SyncState syncState, final MiningParameters miningParameters, final TransactionPoolConfiguration transactionPoolConfiguration, final AbstractPendingTransactionsSorter pendingTransactions, @@ -100,7 +104,7 @@ static TransactionPool createTransactionPool( ethContext.getScheduler(), new TransactionsMessageProcessor(transactionTracker, transactionPool, metricsSystem), transactionPoolConfiguration.getTxMessageKeepAliveSeconds()); - ethContext.getEthMessages().subscribe(EthPV62.TRANSACTIONS, transactionsMessageHandler); + final NewPooledTransactionHashesMessageHandler pooledTransactionsMessageHandler = new NewPooledTransactionHashesMessageHandler( ethContext.getScheduler(), @@ -109,16 +113,47 @@ static TransactionPool createTransactionPool( transactionPool, transactionPoolConfiguration, ethContext, - metricsSystem, - shouldProcessTransactions), + metricsSystem), transactionPoolConfiguration.getTxMessageKeepAliveSeconds()); + + if (syncState.isInitialSyncPhaseDone()) { + enableTransactionPool( + protocolContext, + ethContext, + transactionTracker, + transactionPool, + transactionsMessageHandler, + pooledTransactionsMessageHandler); + } else { + syncState.subscribeCompletionReached( + () -> { + enableTransactionPool( + protocolContext, + ethContext, + transactionTracker, + transactionPool, + transactionsMessageHandler, + pooledTransactionsMessageHandler); + }); + } + + return transactionPool; + } + + private static void enableTransactionPool( + final ProtocolContext protocolContext, + final EthContext ethContext, + final PeerTransactionTracker transactionTracker, + final TransactionPool transactionPool, + final TransactionsMessageHandler transactionsMessageHandler, + final NewPooledTransactionHashesMessageHandler pooledTransactionsMessageHandler) { + LOG.info("Enabling transaction pool"); + ethContext.getEthPeers().subscribeDisconnect(transactionTracker); + protocolContext.getBlockchain().observeBlockAdded(transactionPool); + ethContext.getEthMessages().subscribe(EthPV62.TRANSACTIONS, transactionsMessageHandler); ethContext .getEthMessages() .subscribe(EthPV65.NEW_POOLED_TRANSACTION_HASHES, pooledTransactionsMessageHandler); - - protocolContext.getBlockchain().observeBlockAdded(transactionPool); - ethContext.getEthPeers().subscribeDisconnect(transactionTracker); - return transactionPool; } private static AbstractPendingTransactionsSorter createPendingTransactionsSorter( diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java index aabeae8aa12..0c753597b47 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java @@ -56,6 +56,7 @@ import org.hyperledger.besu.ethereum.eth.messages.ReceiptsMessage; import org.hyperledger.besu.ethereum.eth.messages.StatusMessage; import org.hyperledger.besu.ethereum.eth.messages.TransactionsMessage; +import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolFactory; @@ -1084,7 +1085,7 @@ public void transactionMessagesGoToTheCorrectExecutor() { ethManager.ethContext(), TestClock.system(ZoneId.systemDefault()), metricsSystem, - () -> true, + new SyncState(blockchain, ethManager.ethContext().getEthPeers()), new MiningParameters.Builder().minTransactionGasPrice(Wei.ZERO).build(), TransactionPoolConfiguration.DEFAULT); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java index 18b9f3aae4b..9ab9b1eef29 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java @@ -107,7 +107,7 @@ public void setupTest() { ethContext, TestClock.system(ZoneId.systemDefault()), metricsSystem, - syncState::isInitialSyncPhaseDone, + syncState, new MiningParameters.Builder().minTransactionGasPrice(Wei.ONE).build(), TransactionPoolConfiguration.DEFAULT); ethProtocolManager = diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java index 3545824c0a7..c4b7914c68a 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java @@ -34,7 +34,6 @@ import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; import org.hyperledger.besu.ethereum.eth.messages.NewPooledTransactionHashesMessage; -import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.NewPooledTransactionHashesMessageProcessor.FetcherCreatorTask; import org.hyperledger.besu.metrics.StubMetricsSystem; @@ -57,7 +56,6 @@ public class NewPooledTransactionHashesMessageProcessorTest { @Mock private EthPeer peer1; @Mock private EthContext ethContext; @Mock private EthScheduler ethScheduler; - @Mock private SyncState syncState; private final BlockDataGenerator generator = new BlockDataGenerator(); private final Hash hash1 = generator.transaction().getHash(); @@ -72,15 +70,13 @@ public void setup() { metricsSystem = new StubMetricsSystem(); when(transactionPoolConfiguration.getEth65TrxAnnouncedBufferingPeriod()) .thenReturn(Duration.ofMillis(500)); - when(syncState.isInitialSyncPhaseDone()).thenReturn(true); messageHandler = new NewPooledTransactionHashesMessageProcessor( transactionTracker, transactionPool, transactionPoolConfiguration, ethContext, - metricsSystem, - syncState::isInitialSyncPhaseDone); + metricsSystem); when(ethContext.getScheduler()).thenReturn(ethScheduler); } @@ -187,17 +183,4 @@ public void shouldNotScheduleGetPooledTransactionsTaskTwice() { verify(ethScheduler, times(1)) .scheduleFutureTask(any(FetcherCreatorTask.class), any(Duration.class)); } - - @Test - public void shouldNotAddTransactionsWhenDisabled() { - - when(syncState.isInitialSyncPhaseDone()).thenReturn(false); - messageHandler.processNewPooledTransactionHashesMessage( - peer1, - NewPooledTransactionHashesMessage.create(asList(hash1, hash2, hash3)), - now(), - ofMinutes(1)); - - verifyNoInteractions(transactionPool); - } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java index 515315c4e37..bc8e1db7833 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java @@ -123,6 +123,7 @@ public TestNode( final SyncState syncState = mock(SyncState.class); when(syncState.isInSync(anyLong())).thenReturn(true); + when(syncState.isInitialSyncPhaseDone()).thenReturn(true); final EthMessages ethMessages = new EthMessages(); @@ -144,7 +145,7 @@ public TestNode( ethContext, TestClock.system(ZoneId.systemDefault()), metricsSystem, - syncState::isInitialSyncPhaseDone, + syncState, new MiningParameters.Builder().minTransactionGasPrice(Wei.ZERO).build(), TransactionPoolConfiguration.DEFAULT); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java index 525a5b219e2..7d2fa4f88c6 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java @@ -15,29 +15,31 @@ package org.hyperledger.besu.ethereum.eth.transactions; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; -import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.atLeast; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.ProtocolContext; +import org.hyperledger.besu.ethereum.chain.BlockAddedObserver; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; -import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthMessages; -import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.EthPeers; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; import org.hyperledger.besu.ethereum.eth.manager.ForkIdManager; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; +import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.sorter.GasPricePendingTransactionsSorter; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage; @@ -49,47 +51,173 @@ import java.util.Collections; import java.util.Optional; +import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; -@SuppressWarnings("unchecked") +@RunWith(MockitoJUnitRunner.class) public class TransactionPoolFactoryTest { + @Mock ProtocolSchedule schedule; + @Mock ProtocolContext context; + @Mock MutableBlockchain blockchain; + @Mock EthContext ethContext; + @Mock EthMessages ethMessages; + @Mock EthScheduler ethScheduler; - @Test - public void testDisconnect() { - final ProtocolSchedule schedule = mock(ProtocolSchedule.class); - final ProtocolContext context = mock(ProtocolContext.class); - final MutableBlockchain blockchain = mock(MutableBlockchain.class); + @Mock GasPricePendingTransactionsSorter pendingTransactions; + @Mock PeerTransactionTracker peerTransactionTracker; + @Mock TransactionsMessageSender transactionsMessageSender; + + @Mock NewPooledTransactionHashesMessageSender newPooledTransactionHashesMessageSender; + + TransactionPool pool; + EthPeers ethPeers; + + SyncState syncState; + + EthProtocolManager ethProtocolManager; - when(blockchain.getBlockByNumber(anyLong())).thenReturn(Optional.of(mock(Block.class))); + @Before + public void setup() { when(blockchain.getBlockHashByNumber(anyLong())).thenReturn(Optional.of(mock(Hash.class))); when(context.getBlockchain()).thenReturn(blockchain); - final EthPeers ethPeers = + ethPeers = new EthPeers( "ETH", TestClock.fixed(), new NoOpMetricsSystem(), 25, EthProtocolConfiguration.DEFAULT_MAX_MESSAGE_SIZE); - final EthContext ethContext = mock(EthContext.class); - when(ethContext.getEthMessages()).thenReturn(mock(EthMessages.class)); + when(ethContext.getEthMessages()).thenReturn(ethMessages); when(ethContext.getEthPeers()).thenReturn(ethPeers); - final EthScheduler ethScheduler = mock(EthScheduler.class); + when(ethContext.getScheduler()).thenReturn(ethScheduler); - final GasPricePendingTransactionsSorter pendingTransactions = - mock(GasPricePendingTransactionsSorter.class); - final PeerTransactionTracker peerTransactionTracker = mock(PeerTransactionTracker.class); - final TransactionsMessageSender transactionsMessageSender = - mock(TransactionsMessageSender.class); - doNothing().when(transactionsMessageSender).sendTransactionsToPeer(any(EthPeer.class)); - final NewPooledTransactionHashesMessageSender newPooledTransactionHashesMessageSender = - mock(NewPooledTransactionHashesMessageSender.class); - final TransactionPool pool = + } + + @Test + public void disconnectNotInvokedBeforeInitialSyncIsDone() { + setupInitialSyncPhase(true); + final RespondingEthPeer ethPeer = + RespondingEthPeer.builder().ethProtocolManager(ethProtocolManager).build(); + assertThat(ethPeer.getEthPeer()).isNotNull(); + assertThat(ethPeer.getEthPeer().isDisconnected()).isFalse(); + ethPeer.disconnect(DisconnectMessage.DisconnectReason.CLIENT_QUITTING); + verifyNoInteractions(peerTransactionTracker); + } + + @Test + public void disconnectInvokedAfterInitialSyncIsDone() { + setupInitialSyncPhase(true); + final RespondingEthPeer ethPeer = + RespondingEthPeer.builder().ethProtocolManager(ethProtocolManager).build(); + assertThat(ethPeer.getEthPeer()).isNotNull(); + assertThat(ethPeer.getEthPeer().isDisconnected()).isFalse(); + + syncState.markInitialSyncPhaseAsDone(); + + ethPeer.disconnect(DisconnectMessage.DisconnectReason.CLIENT_QUITTING); + verify(peerTransactionTracker, times(1)).onDisconnect(ethPeer.getEthPeer()); + } + + @Test + public void disconnectInvokedIfNoInitialSync() { + setupInitialSyncPhase(false); + final RespondingEthPeer ethPeer = + RespondingEthPeer.builder().ethProtocolManager(ethProtocolManager).build(); + assertThat(ethPeer.getEthPeer()).isNotNull(); + assertThat(ethPeer.getEthPeer().isDisconnected()).isFalse(); + + ethPeer.disconnect(DisconnectMessage.DisconnectReason.CLIENT_QUITTING); + verify(peerTransactionTracker, times(1)).onDisconnect(ethPeer.getEthPeer()); + } + + @Test + public void notRegisteredToBlockAddedEventBeforeInitialSyncIsDone() { + setupInitialSyncPhase(true); + ArgumentCaptor blockAddedListeners = + ArgumentCaptor.forClass(BlockAddedObserver.class); + verify(blockchain, atLeastOnce()).observeBlockAdded(blockAddedListeners.capture()); + + assertThat(blockAddedListeners.getAllValues()).doesNotContain(pool); + } + + @Test + public void registeredToBlockAddedEventAfterInitialSyncIsDone() { + setupInitialSyncPhase(true); + syncState.markInitialSyncPhaseAsDone(); + + ArgumentCaptor blockAddedListeners = + ArgumentCaptor.forClass(BlockAddedObserver.class); + verify(blockchain, atLeastOnce()).observeBlockAdded(blockAddedListeners.capture()); + + assertThat(blockAddedListeners.getAllValues()).contains(pool); + } + + @Test + public void registeredToBlockAddedEventIfNoInitialSync() { + setupInitialSyncPhase(false); + + ArgumentCaptor blockAddedListeners = + ArgumentCaptor.forClass(BlockAddedObserver.class); + verify(blockchain, atLeastOnce()).observeBlockAdded(blockAddedListeners.capture()); + + assertThat(blockAddedListeners.getAllValues()).contains(pool); + } + + @Test + public void incomingTransactionMessageHandlersNotRegisteredBeforeInitialSyncIsDone() { + setupInitialSyncPhase(true); + ArgumentCaptor messageHandlers = + ArgumentCaptor.forClass(EthMessages.MessageCallback.class); + verify(ethMessages, atLeast(0)).subscribe(anyInt(), messageHandlers.capture()); + + assertThat(messageHandlers.getAllValues()) + .doesNotHaveAnyElementsOfTypes( + TransactionsMessageHandler.class, NewPooledTransactionHashesMessageHandler.class); + } + + @Test + public void incomingTransactionMessageHandlersRegisteredAfterInitialSyncIsDone() { + setupInitialSyncPhase(true); + syncState.markInitialSyncPhaseAsDone(); + + ArgumentCaptor messageHandlers = + ArgumentCaptor.forClass(EthMessages.MessageCallback.class); + verify(ethMessages, atLeast(0)).subscribe(anyInt(), messageHandlers.capture()); + + assertThat(messageHandlers.getAllValues()) + .hasAtLeastOneElementOfType(TransactionsMessageHandler.class); + assertThat(messageHandlers.getAllValues()) + .hasAtLeastOneElementOfType(NewPooledTransactionHashesMessageHandler.class); + } + + @Test + public void incomingTransactionMessageHandlersRegisteredIfNoInitialSync() { + setupInitialSyncPhase(false); + + ArgumentCaptor messageHandlers = + ArgumentCaptor.forClass(EthMessages.MessageCallback.class); + verify(ethMessages, atLeast(0)).subscribe(anyInt(), messageHandlers.capture()); + + assertThat(messageHandlers.getAllValues()) + .hasAtLeastOneElementOfType(TransactionsMessageHandler.class); + assertThat(messageHandlers.getAllValues()) + .hasAtLeastOneElementOfType(NewPooledTransactionHashesMessageHandler.class); + } + + private void setupInitialSyncPhase(final boolean hasInitialSyncPhase) { + syncState = new SyncState(blockchain, ethPeers, hasInitialSyncPhase, Optional.empty()); + + pool = TransactionPoolFactory.createTransactionPool( schedule, context, ethContext, new NoOpMetricsSystem(), - () -> true, + syncState, new MiningParameters.Builder().minTransactionGasPrice(Wei.ONE).build(), ImmutableTransactionPoolConfiguration.builder() .txPoolMaxSize(1) @@ -101,7 +229,7 @@ public void testDisconnect() { transactionsMessageSender, newPooledTransactionHashesMessageSender); - final EthProtocolManager ethProtocolManager = + ethProtocolManager = new EthProtocolManager( blockchain, BigInteger.ONE, @@ -116,12 +244,5 @@ public void testDisconnect() { true, mock(EthScheduler.class), mock(ForkIdManager.class)); - - final RespondingEthPeer ethPeer = - RespondingEthPeer.builder().ethProtocolManager(ethProtocolManager).build(); - assertThat(ethPeer.getEthPeer()).isNotNull(); - assertThat(ethPeer.getEthPeer().isDisconnected()).isFalse(); - ethPeer.disconnect(DisconnectMessage.DisconnectReason.CLIENT_QUITTING); - verify(peerTransactionTracker, times(1)).onDisconnect(ethPeer.getEthPeer()); } } diff --git a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java index fdd6170949d..2d31ef0aee8 100644 --- a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java +++ b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java @@ -213,7 +213,7 @@ private boolean buildContext( ethContext, retestethClock, metricsSystem, - syncState::isInitialSyncPhaseDone, + syncState, new MiningParameters.Builder().minTransactionGasPrice(Wei.ZERO).build(), transactionPoolConfiguration); diff --git a/evm/src/main/java/org/hyperledger/besu/evm/gascalculator/TangerineWhistleGasCalculator.java b/evm/src/main/java/org/hyperledger/besu/evm/gascalculator/TangerineWhistleGasCalculator.java index df05e9e8742..d98674dc07b 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/gascalculator/TangerineWhistleGasCalculator.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/gascalculator/TangerineWhistleGasCalculator.java @@ -20,6 +20,7 @@ import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.evm.account.Account; import org.hyperledger.besu.evm.frame.MessageFrame; +import org.hyperledger.besu.evm.internal.Words; public class TangerineWhistleGasCalculator extends HomesteadGasCalculator { @@ -82,7 +83,7 @@ public long callOperationGasCost( } private static long gasCap(final long remaining, final long stipend) { - return Math.min(allButOneSixtyFourth(remaining), stipend); + return Words.unsignedMin(allButOneSixtyFourth(remaining), stipend); } @Override diff --git a/evm/src/main/java/org/hyperledger/besu/evm/internal/Words.java b/evm/src/main/java/org/hyperledger/besu/evm/internal/Words.java index c95094e8ba8..f161ab9d748 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/internal/Words.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/internal/Words.java @@ -134,4 +134,15 @@ public static long clampedMultiply(final long a, final long b) { return ((a ^ b) < 0) ? Long.MIN_VALUE : Long.MAX_VALUE; } } + + /** + * Returns the lesser of the two values, when compared as an unsigned value + * + * @param a first value + * @param b second value + * @return a if, as an unsigned integer, a is less than b; otherwise b. + */ + public static long unsignedMin(final long a, final long b) { + return Long.compareUnsigned(a, b) < 0 ? a : b; + } } diff --git a/evm/src/test/java/org/hyperledger/besu/evm/internal/WordsTest.java b/evm/src/test/java/org/hyperledger/besu/evm/internal/WordsTest.java new file mode 100644 index 00000000000..40eb0a0b898 --- /dev/null +++ b/evm/src/test/java/org/hyperledger/besu/evm/internal/WordsTest.java @@ -0,0 +1,52 @@ +/* + * Copyright contributors to Hyperledger Besu + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * + */ + +package org.hyperledger.besu.evm.internal; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.hyperledger.besu.evm.internal.Words.unsignedMin; + +import java.util.Arrays; +import java.util.Collection; + +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.TestInstance.Lifecycle; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +@TestInstance(Lifecycle.PER_CLASS) +class WordsTest { + + Collection unsignedMinLongTestVector() { + return Arrays.asList( + new Object[][] { + {10, 10, 10}, + {-10, -10, -10}, + {10, 20, 10}, + {20, 10, 10}, + {10, -10, 10}, + {-10, 10, 10}, + {-20, -10, -20}, + {-10, -20, -20}, + }); + } + + @ParameterizedTest + @MethodSource("unsignedMinLongTestVector") + void unsugnedMinLongTest(final long a, final long b, final long min) { + assertThat(unsignedMin(a, b)).isEqualTo(min); + } +} diff --git a/gradle.properties b/gradle.properties index ed0e618af8b..0c60687fedf 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,4 @@ -version=22.7.5-SNAPSHOT +version=22.7.5 org.gradle.welcome=never org.gradle.jvmargs=-Xmx1g