Skip to content

Commit

Permalink
add parent beacon block root to payload id calculation (hyperledger#5843
Browse files Browse the repository at this point in the history 
)

Signed-off-by: Stefan <stefan.pingel@consensys.net>
  • Loading branch information
@pinges @NickSneo
pinges authored and NickSneo committed Nov 12, 2023
1 parent eafd5f3 commit 8deacf7
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 13 deletions.
7 changes: 6 additions & 1 deletion ...ge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,12 @@ public PayloadIdentifier preparePayload(

final PayloadIdentifier payloadIdentifier =
PayloadIdentifier.forPayloadParams(
parentHeader.getBlockHash(), timestamp, prevRandao, feeRecipient, withdrawals);
parentHeader.getBlockHash(),
timestamp,
prevRandao,
feeRecipient,
withdrawals,
parentBeaconBlockRoot);

if (blockCreationTasks.containsKey(payloadIdentifier)) {
LOG.debug(
Expand Down
7 changes: 5 additions & 2 deletions ...e/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadIdentifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,16 @@ public PayloadIdentifier(final Long payloadId) {
* @param prevRandao the prev randao
* @param feeRecipient the fee recipient
* @param withdrawals the withdrawals
* @param parentBeaconBlockRoot the parent beacon block root
* @return the payload identifier
*/
public static PayloadIdentifier forPayloadParams(
final Hash parentHash,
final Long timestamp,
final Bytes32 prevRandao,
final Address feeRecipient,
final Optional<List<Withdrawal>> withdrawals) {
final Optional<List<Withdrawal>> withdrawals,
final Optional<Bytes32> parentBeaconBlockRoot) {

return new PayloadIdentifier(
timestamp
Expand All @@ -84,7 +86,8 @@ public static PayloadIdentifier forPayloadParams(
.sorted(Comparator.comparing(Withdrawal::getIndex))
.map(Withdrawal::hashCode)
.reduce(1, (a, b) -> a ^ (b * 31)))
.orElse(0));
.orElse(0)
^ ((long) parentBeaconBlockRoot.hashCode()) << 40);
}

@Override
Expand Down
93 changes: 86 additions & 7 deletions ...c/test/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadIdentifierTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@ public void serializesToEvenHexRepresentation() {
public void conversionCoverage() {
var idTest =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, Bytes32.random(), Address.fromHexString("0x42"), Optional.empty());
Hash.ZERO,
1337L,
Bytes32.random(),
Address.fromHexString("0x42"),
Optional.empty(),
Optional.empty());
assertThat(new PayloadIdentifier(idTest.getAsBigInteger().longValue())).isEqualTo(idTest);
assertThat(new PayloadIdentifier(idTest.getAsBigInteger().longValue())).isEqualTo(idTest);
}
Expand Down Expand Up @@ -82,10 +87,20 @@ public void differentWithdrawalAmountsYieldDifferentHash() {
final Bytes32 prevRandao = Bytes32.random();
var idForWithdrawals1 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, prevRandao, Address.fromHexString("0x42"), Optional.of(withdrawals1));
Hash.ZERO,
1337L,
prevRandao,
Address.fromHexString("0x42"),
Optional.of(withdrawals1),
Optional.empty());
var idForWithdrawals2 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, prevRandao, Address.fromHexString("0x42"), Optional.of(withdrawals2));
Hash.ZERO,
1337L,
prevRandao,
Address.fromHexString("0x42"),
Optional.of(withdrawals2),
Optional.empty());
assertThat(idForWithdrawals1).isNotEqualTo(idForWithdrawals2);
}

Expand Down Expand Up @@ -118,10 +133,20 @@ public void differentOrderedWithdrawalsYieldSameHash() {
final Bytes32 prevRandao = Bytes32.random();
var idForWithdrawals1 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, prevRandao, Address.fromHexString("0x42"), Optional.of(withdrawals1));
Hash.ZERO,
1337L,
prevRandao,
Address.fromHexString("0x42"),
Optional.of(withdrawals1),
Optional.empty());
var idForWithdrawals2 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, prevRandao, Address.fromHexString("0x42"), Optional.of(withdrawals2));
Hash.ZERO,
1337L,
prevRandao,
Address.fromHexString("0x42"),
Optional.of(withdrawals2),
Optional.empty());
assertThat(idForWithdrawals1).isEqualTo(idForWithdrawals2);
}

Expand All @@ -130,10 +155,64 @@ public void emptyOptionalAndEmptyListWithdrawalsYieldDifferentHash() {
final Bytes32 prevRandao = Bytes32.random();
var idForWithdrawals1 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, prevRandao, Address.fromHexString("0x42"), Optional.empty());
Hash.ZERO,
1337L,
prevRandao,
Address.fromHexString("0x42"),
Optional.empty(),
Optional.empty());
var idForWithdrawals2 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, prevRandao, Address.fromHexString("0x42"), Optional.of(emptyList()));
Hash.ZERO,
1337L,
prevRandao,
Address.fromHexString("0x42"),
Optional.of(emptyList()),
Optional.empty());
assertThat(idForWithdrawals1).isNotEqualTo(idForWithdrawals2);
}

@Test
public void emptyOptionalAndNonEmptyParentBeaconBlockRootYieldDifferentHash() {
final Bytes32 prevRandao = Bytes32.random();
var idForWithdrawals1 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO,
1337L,
prevRandao,
Address.fromHexString("0x42"),
Optional.empty(),
Optional.empty());
var idForWithdrawals2 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO,
1337L,
prevRandao,
Address.fromHexString("0x42"),
Optional.empty(),
Optional.of(Bytes32.ZERO));
assertThat(idForWithdrawals1).isNotEqualTo(idForWithdrawals2);
}

@Test
public void differentParentBeaconBlockRootYieldDifferentHash() {
final Bytes32 prevRandao = Bytes32.random();
var idForWithdrawals1 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO,
1337L,
prevRandao,
Address.fromHexString("0x42"),
Optional.empty(),
Optional.of(Bytes32.fromHexStringLenient("0x1")));
var idForWithdrawals2 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO,
1337L,
prevRandao,
Address.fromHexString("0x42"),
Optional.empty(),
Optional.of(Bytes32.ZERO));
assertThat(idForWithdrawals1).isNotEqualTo(idForWithdrawals2);
}
}
6 changes: 5 additions & 1 deletion ...esu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ public void shouldReturnValidWithoutFinalizedWithPayload() {
payloadParams.getTimestamp(),
payloadParams.getPrevRandao(),
payloadParams.getSuggestedFeeRecipient(),
Optional.empty(),
Optional.empty());

when(mergeCoordinator.preparePayload(
Expand Down Expand Up @@ -518,6 +519,7 @@ public void shouldReturnValidIfWithdrawalsIsNull_WhenWithdrawalsProhibited() {
payloadParams.getTimestamp(),
payloadParams.getPrevRandao(),
payloadParams.getSuggestedFeeRecipient(),
Optional.empty(),
Optional.empty());

when(mergeCoordinator.preparePayload(
Expand Down Expand Up @@ -603,7 +605,8 @@ public void shouldReturnValidIfWithdrawalsIsNotNull_WhenWithdrawalsAllowed() {
payloadParams.getTimestamp(),
payloadParams.getPrevRandao(),
payloadParams.getSuggestedFeeRecipient(),
withdrawals);
withdrawals,
Optional.empty());

when(mergeCoordinator.preparePayload(
mockHeader,
Expand Down Expand Up @@ -645,6 +648,7 @@ public void shouldReturnValidIfProtocolScheduleIsEmpty() {
payloadParams.getTimestamp(),
payloadParams.getPrevRandao(),
payloadParams.getSuggestedFeeRecipient(),
Optional.empty(),
Optional.empty());

when(mergeCoordinator.preparePayload(
Expand Down
14 changes: 12 additions & 2 deletions ...edger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineGetPayloadTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,12 @@ public AbstractEngineGetPayloadTest() {
protected static final BlockResultFactory factory = new BlockResultFactory();
protected static final PayloadIdentifier mockPid =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, Bytes32.random(), Address.fromHexString("0x42"), Optional.empty());
Hash.ZERO,
1337L,
Bytes32.random(),
Address.fromHexString("0x42"),
Optional.empty(),
Optional.empty());
protected static final BlockHeader mockHeader =
new BlockHeaderTestFixture().prevRandao(Bytes32.random()).buildHeader();
private static final Block mockBlock =
Expand Down Expand Up @@ -147,7 +152,12 @@ public void shouldFailForUnknownPayloadId() {
resp(
getMethodName(),
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 0L, Bytes32.random(), Address.fromHexString("0x42"), Optional.empty()));
Hash.ZERO,
0L,
Bytes32.random(),
Address.fromHexString("0x42"),
Optional.empty(),
Optional.empty()));
assertThat(resp).isInstanceOf(JsonRpcErrorResponse.class);
verify(engineCallListener, times(1)).executionEngineCalled();
}
Expand Down
1 change: 1 addition & 0 deletions ...hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ public void shouldReturnBlockForKnownPayloadId() {
cancunHardfork.milestone(),
Bytes32.random(),
Address.fromHexString("0x42"),
Optional.empty(),
Optional.empty());

BlobTestFixture blobTestFixture = new BlobTestFixture();
Expand Down

0 comments on commit 8deacf7

Please sign in to comment.