Skip to content

Commit

Permalink
Improve the selection of the most profitable built block
Browse files Browse the repository at this point in the history 
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
  • Loading branch information
@fab-10
fab-10 committed Jun 5, 2024
1 parent e4daf6a commit d24a505
Show file tree
Hide file tree
Showing 17 changed files with 187 additions and 159 deletions.
3 changes: 1 addition & 2 deletions consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.ethereum.ConsensusContext;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
import org.hyperledger.besu.ethereum.core.Difficulty;
import org.hyperledger.besu.ethereum.eth.sync.state.SyncState;

Expand Down Expand Up @@ -167,7 +166,7 @@ void fireNewUnverifiedForkchoiceEvent(
* @param payloadId the payload identifier
* @return the optional block with receipts
*/
Optional<BlockWithReceipts> retrieveBlockById(final PayloadIdentifier payloadId);
Optional<PayloadWrapper> retrievePayloadById(final PayloadIdentifier payloadId);

/**
* Is configured for a post-merge from genesis.
Expand Down
54 changes: 46 additions & 8 deletions consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,51 @@
package org.hyperledger.besu.consensus.merge;

import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.core.BlockValueCalculator;
import org.hyperledger.besu.ethereum.core.BlockWithReceipts;

/**
* Wrapper for payload plus extra info.
*
* @param payloadIdentifier Payload identifier
* @param blockWithReceipts Block With Receipts
*/
public record PayloadWrapper(
PayloadIdentifier payloadIdentifier, BlockWithReceipts blockWithReceipts) {}
/** Wrapper for payload plus extra info. */
public class PayloadWrapper {
private final PayloadIdentifier payloadIdentifier;
private final BlockWithReceipts blockWithReceipts;
private final Wei blockValue;

/**
* @param payloadIdentifier Payload identifier
* @param blockWithReceipts Block with receipts
*/
public PayloadWrapper(
final PayloadIdentifier payloadIdentifier, final BlockWithReceipts blockWithReceipts) {
this.blockWithReceipts = blockWithReceipts;
this.payloadIdentifier = payloadIdentifier;
this.blockValue = BlockValueCalculator.calculateBlockValue(blockWithReceipts);
}

/**
* Get the block value
*
* @return block value in Wei
*/
public Wei blockValue() {
return blockValue;
}

/**
* Get this payload identifier
*
* @return payload identifier
*/
public PayloadIdentifier payloadIdentifier() {
return payloadIdentifier;
}

/**
* Get the block with receipts
*
* @return block with receipts
*/
public BlockWithReceipts blockWithReceipts() {
return blockWithReceipts;
}
}
77 changes: 32 additions & 45 deletions consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,16 @@

import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.ConsensusContext;
import org.hyperledger.besu.ethereum.core.Block;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.BlockValueCalculator;
import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
import org.hyperledger.besu.ethereum.core.Difficulty;
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;
import java.util.stream.Stream;

import com.google.common.annotations.VisibleForTesting;
Expand All @@ -45,13 +41,6 @@ public class PostMergeContext implements MergeContext {
static final int MAX_BLOCKS_IN_PROGRESS = 12;

private static final AtomicReference<PostMergeContext> singleton = new AtomicReference<>();

private static final Comparator<BlockWithReceipts> compareByGasUsedDesc =
Comparator.comparingLong(
(BlockWithReceipts blockWithReceipts) ->
blockWithReceipts.getBlock().getHeader().getGasUsed())
.reversed();

private final AtomicReference<SyncState> syncState;
private final AtomicReference<Difficulty> terminalTotalDifficulty;
// initial postMerge state is indeterminate until it is set:
Expand All @@ -70,7 +59,6 @@ public class PostMergeContext implements MergeContext {
private final AtomicReference<BlockHeader> lastSafeBlock = new AtomicReference<>();
private final AtomicReference<Optional<BlockHeader>> terminalPoWBlock =
new AtomicReference<>(Optional.empty());
private final BlockValueCalculator blockValueCalculator = new BlockValueCalculator();
private boolean isPostMergeAtGenesis;

/** Instantiates a new Post merge context. */
Expand Down Expand Up @@ -227,66 +215,65 @@ public boolean validateCandidateHead(final BlockHeader candidateHeader) {
}

@Override
public void putPayloadById(final PayloadWrapper payloadWrapper) {
public void putPayloadById(final PayloadWrapper newPayload) {
final var newBlockWithReceipts = newPayload.blockWithReceipts();
final var newBlockValue = newPayload.blockValue();

synchronized (blocksInProgress) {
final Optional<BlockWithReceipts> maybeCurrBestBlock =
retrieveBlockById(payloadWrapper.payloadIdentifier());
final Optional<PayloadWrapper> maybeCurrBestPayload =
retrievePayloadById(newPayload.payloadIdentifier());

maybeCurrBestBlock.ifPresentOrElse(
currBestBlock -> {
if (compareByGasUsedDesc.compare(payloadWrapper.blockWithReceipts(), currBestBlock)
< 0) {
maybeCurrBestPayload.ifPresent(
currBestPayload -> {
if (newBlockValue.greaterThan(currBestPayload.blockValue())) {
LOG.atDebug()
.setMessage("New proposal for payloadId {} {} is better than the previous one {}")
.addArgument(payloadWrapper.payloadIdentifier())
.setMessage(
"New proposal for payloadId {} {} is better than the previous one {} by {}")
.addArgument(newPayload.payloadIdentifier())
.addArgument(() -> logBlockProposal(newBlockWithReceipts.getBlock()))
.addArgument(
() -> logBlockProposal(payloadWrapper.blockWithReceipts().getBlock()))
.addArgument(() -> logBlockProposal(currBestBlock.getBlock()))
() -> logBlockProposal(currBestPayload.blockWithReceipts().getBlock()))
.addArgument(
() ->
newBlockValue
.subtract(currBestPayload.blockValue())
.toHumanReadableString())
.log();

blocksInProgress.removeAll(
retrievePayloadsById(payloadWrapper.payloadIdentifier())
.collect(Collectors.toUnmodifiableList()));
blocksInProgress.add(
new PayloadWrapper(
payloadWrapper.payloadIdentifier(), payloadWrapper.blockWithReceipts()));
logCurrentBestBlock(payloadWrapper.blockWithReceipts());
streamPayloadsById(newPayload.payloadIdentifier()).toList());

logCurrentBestBlock(newPayload);
}
},
() ->
blocksInProgress.add(
new PayloadWrapper(
payloadWrapper.payloadIdentifier(), payloadWrapper.blockWithReceipts())));
});
blocksInProgress.add(newPayload);
}
}

private void logCurrentBestBlock(final BlockWithReceipts blockWithReceipts) {
private void logCurrentBestBlock(final PayloadWrapper payloadWrapper) {
if (LOG.isDebugEnabled()) {
final Block block = blockWithReceipts.getBlock();
final Block block = payloadWrapper.blockWithReceipts().getBlock();
final float gasUsedPerc =
100.0f * block.getHeader().getGasUsed() / block.getHeader().getGasLimit();
final int txsNum = block.getBody().getTransactions().size();
final Wei reward = blockValueCalculator.calculateBlockValue(blockWithReceipts);

LOG.debug(
"Current best proposal for block {}: txs {}, gas used {}%, reward {}",
blockWithReceipts.getNumber(),
block.getHeader().getNumber(),
txsNum,
String.format("%1.2f", gasUsedPerc),
reward.toHumanReadableString());
payloadWrapper.blockValue().toHumanReadableString());
}
}

@Override
public Optional<BlockWithReceipts> retrieveBlockById(final PayloadIdentifier payloadId) {
public Optional<PayloadWrapper> retrievePayloadById(final PayloadIdentifier payloadId) {
synchronized (blocksInProgress) {
return retrievePayloadsById(payloadId)
.map(payloadWrapper -> payloadWrapper.blockWithReceipts())
.sorted(compareByGasUsedDesc)
.findFirst();
return streamPayloadsById(payloadId).max(Comparator.comparing(PayloadWrapper::blockValue));
}
}

private Stream<PayloadWrapper> retrievePayloadsById(final PayloadIdentifier payloadId) {
private Stream<PayloadWrapper> streamPayloadsById(final PayloadIdentifier payloadId) {
return blocksInProgress.stream().filter(z -> z.payloadIdentifier().equals(payloadId));
}

Expand Down
5 changes: 2 additions & 3 deletions consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.ethereum.ConsensusContext;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
import org.hyperledger.besu.ethereum.core.Difficulty;
import org.hyperledger.besu.ethereum.eth.sync.state.SyncState;

Expand Down Expand Up @@ -146,8 +145,8 @@ public void putPayloadById(final PayloadWrapper payloadWrapper) {
}

@Override
public Optional<BlockWithReceipts> retrieveBlockById(final PayloadIdentifier payloadId) {
return postMergeContext.retrieveBlockById(payloadId);
public Optional<PayloadWrapper> retrievePayloadById(final PayloadIdentifier payloadId) {
return postMergeContext.retrievePayloadById(payloadId);
}

@Override
Expand Down
55 changes: 42 additions & 13 deletions consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.core.Block;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
Expand Down Expand Up @@ -138,9 +139,12 @@ public void putAndRetrieveFirstPayload() {
BlockWithReceipts mockBlockWithReceipts = createBlockWithReceipts(1, 21000, 1);

PayloadIdentifier firstPayloadId = new PayloadIdentifier(1L);
postMergeContext.putPayloadById(new PayloadWrapper(firstPayloadId, mockBlockWithReceipts));
final var payloadWrapper = createPayloadWrapper(firstPayloadId, mockBlockWithReceipts, Wei.ONE);
postMergeContext.putPayloadById(payloadWrapper);

assertThat(postMergeContext.retrieveBlockById(firstPayloadId)).contains(mockBlockWithReceipts);
assertThat(postMergeContext.retrievePayloadById(firstPayloadId))
.map(PayloadWrapper::blockWithReceipts)
.contains(mockBlockWithReceipts);
}

@Test
Expand All @@ -149,10 +153,16 @@ public void puttingTwoBlocksWithTheSamePayloadIdWeRetrieveTheBest() {
BlockWithReceipts betterBlockWithReceipts = createBlockWithReceipts(2, 11, 1);

PayloadIdentifier payloadId = new PayloadIdentifier(1L);
postMergeContext.putPayloadById(new PayloadWrapper(payloadId, zeroTxBlockWithReceipts));
postMergeContext.putPayloadById(new PayloadWrapper(payloadId, betterBlockWithReceipts));

assertThat(postMergeContext.retrieveBlockById(payloadId)).contains(betterBlockWithReceipts);
final var zeroTxPayloadWrapper =
createPayloadWrapper(payloadId, zeroTxBlockWithReceipts, Wei.ZERO);
final var betterPayloadWrapper =
createPayloadWrapper(payloadId, betterBlockWithReceipts, Wei.ONE);
postMergeContext.putPayloadById(zeroTxPayloadWrapper);
postMergeContext.putPayloadById(betterPayloadWrapper);

assertThat(postMergeContext.retrievePayloadById(payloadId))
.map(PayloadWrapper::blockWithReceipts)
.contains(betterBlockWithReceipts);
}

@Test
Expand All @@ -162,25 +172,33 @@ public void puttingABlockWithTheSamePayloadIdSmallerThanAnExistingOneWeRetrieveT
BlockWithReceipts smallBlockWithReceipts = createBlockWithReceipts(3, 5, 1);

PayloadIdentifier payloadId = new PayloadIdentifier(1L);
postMergeContext.putPayloadById(new PayloadWrapper(payloadId, zeroTxBlockWithReceipts));
postMergeContext.putPayloadById(new PayloadWrapper(payloadId, betterBlockWithReceipts));
postMergeContext.putPayloadById(new PayloadWrapper(payloadId, smallBlockWithReceipts));

assertThat(postMergeContext.retrieveBlockById(payloadId)).contains(betterBlockWithReceipts);
final var zeroTxPayloadWrapper =
createPayloadWrapper(payloadId, zeroTxBlockWithReceipts, Wei.ZERO);
final var betterPayloadWrapper =
createPayloadWrapper(payloadId, betterBlockWithReceipts, Wei.of(2));
final var smallPayloadWrapper =
createPayloadWrapper(payloadId, smallBlockWithReceipts, Wei.ONE);
postMergeContext.putPayloadById(zeroTxPayloadWrapper);
postMergeContext.putPayloadById(betterPayloadWrapper);
postMergeContext.putPayloadById(smallPayloadWrapper);

assertThat(postMergeContext.retrievePayloadById(payloadId))
.map(PayloadWrapper::blockWithReceipts)
.contains(betterBlockWithReceipts);
}

@Test
public void tryingToRetrieveANotYetPutPayloadIdReturnsEmpty() {
PayloadIdentifier payloadId = new PayloadIdentifier(1L);

assertThat(postMergeContext.retrieveBlockById(payloadId)).isEmpty();
assertThat(postMergeContext.retrievePayloadById(payloadId)).isEmpty();
}

@Test
public void tryingToRetrieveABlockPutButEvictedReturnsEmpty() {
PayloadIdentifier evictedPayloadId = new PayloadIdentifier(0L);

assertThat(postMergeContext.retrieveBlockById(evictedPayloadId)).isEmpty();
assertThat(postMergeContext.retrievePayloadById(evictedPayloadId)).isEmpty();
}

@Test
Expand Down Expand Up @@ -209,6 +227,17 @@ public void syncStateNullShouldNotThrowWhenIsSyncingIsCalled() {
assertThat(postMergeContext.isSyncing()).isFalse();
}

private PayloadWrapper createPayloadWrapper(
final PayloadIdentifier firstPayloadId,
final BlockWithReceipts mockBlockWithReceipts,
final Wei blockValue) {
final var payloadWrapper = mock(PayloadWrapper.class);
when(payloadWrapper.payloadIdentifier()).thenReturn(firstPayloadId);
when(payloadWrapper.blockWithReceipts()).thenReturn(mockBlockWithReceipts);
when(payloadWrapper.blockValue()).thenReturn(blockValue);
return payloadWrapper;
}

private static BlockWithReceipts createBlockWithReceipts(
final int number, final long gasUsed, final int txCount) {
Block mockBlock = mock(Block.class, RETURNS_DEEP_STUBS);
Expand Down
Loading

0 comments on commit d24a505

Please sign in to comment.