Add withdrawals to PayloadIdentifier to avoid collisions (#5321)

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
pull/5325/head
Simon Dudley 2 years ago committed by GitHub
parent f528f0d55a
commit ef54beeb6c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 1
      CHANGELOG.md
  2. 4
      acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/11_shanghai_prepare_payload.json
  3. 46
      acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/12_shanghai_prepare_payload_replay.json
  4. 12
      acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/13_shanghai_get_payload.json
  5. 0
      acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/14_shanghai_invalid_null_withdrawals_execute_payload.json
  6. 6
      acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/15_shanghai_execute_payload.json
  7. 28
      acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/15_shanghai_update_forkchoice.json
  8. 28
      acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/16_shanghai_update_forkchoice.json
  9. 0
      acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/17_shanghai_withdrawals_address1_latest_balance.json
  10. 0
      acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/18_shanghai_withdrawals_address2_latest_balance.json
  11. 2
      consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java
  12. 23
      consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadIdentifier.java
  13. 91
      consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadIdentifierTest.java
  14. 28
      ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java
  15. 4
      ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineGetPayloadTest.java

@ -28,6 +28,7 @@
- Make QBFT validator smart contract mode work with london fork [#5249](https://github.com/hyperledger/besu/issues/5249)
- Try to connect to EthStats server by default with ssl followed by non-ssl. [#5301](https://github.com/hyperledger/besu/pull/5301)
- Allow --miner-extra-data to be used in Proof-of-Stake block production [#5291](https://github.com/hyperledger/besu/pull/5291)
- Add withdrawals to payloadId calculation to avoid collisions [#5321](https://github.com/hyperledger/besu/pull/5321)
### Download Links

@ -21,7 +21,7 @@
},
{
"index": "0x1",
"validatorIndex": "0x0",
"validatorIndex": "0x1",
"address": "0xfe3b557e8fb62b89f4916b721be55ceb828dbd73",
"amount": "0x2"
}
@ -39,7 +39,7 @@
"latestValidHash": "0x3559e851470f6e7bbed1db474980683e8c315bfce99b2a6ef47c057c04de7858",
"validationError": null
},
"payloadId": "0x0065bd32d4f95410"
"payloadId": "0x0065bd329c251cd9"
}
},
"statusCode" : 200

@ -0,0 +1,46 @@
{
"request": {
"jsonrpc": "2.0",
"method": "engine_forkchoiceUpdatedV2",
"params": [
{
"headBlockHash": "0x3559e851470f6e7bbed1db474980683e8c315bfce99b2a6ef47c057c04de7858",
"safeBlockHash": "0x3559e851470f6e7bbed1db474980683e8c315bfce99b2a6ef47c057c04de7858",
"finalizedBlockHash": "0x0000000000000000000000000000000000000000000000000000000000000000"
},
{
"timestamp": "0x10",
"prevRandao": "0x0000000000000000000000000000000000000000000000000000000000000000",
"suggestedFeeRecipient": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b",
"withdrawals": [
{
"index": "0x0",
"validatorIndex": "0x0",
"address": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b",
"amount": "0x3"
},
{
"index": "0x1",
"validatorIndex": "0x1",
"address": "0xfe3b557e8fb62b89f4916b721be55ceb828dbd73",
"amount": "0x4"
}
]
}
],
"id": 67
},
"response": {
"jsonrpc": "2.0",
"id": 67,
"result": {
"payloadStatus": {
"status": "VALID",
"latestValidHash": "0x3559e851470f6e7bbed1db474980683e8c315bfce99b2a6ef47c057c04de7858",
"validationError": null
},
"payloadId": "0x0065bd329c251d59"
}
},
"statusCode" : 200
}

@ -3,7 +3,7 @@
"jsonrpc": "2.0",
"method": "engine_getPayloadV2",
"params": [
"0x0065bd32d4f95410"
"0x0065bd329c251d59"
],
"id": 67
},
@ -14,7 +14,7 @@
"executionPayload": {
"parentHash": "0x3559e851470f6e7bbed1db474980683e8c315bfce99b2a6ef47c057c04de7858",
"feeRecipient": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b",
"stateRoot": "0x7a6f2e03f2348dc75731e6e767c97a88da50430748a030874e27f7c3fac3d49d",
"stateRoot": "0xc8c8e840369eac89a610bfe2ec21fcdee4c9c43bec4876f0129fcd4b5311f6dd",
"logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
"prevRandao": "0x0000000000000000000000000000000000000000000000000000000000000000",
"gasLimit": "0x1c9c380",
@ -28,17 +28,17 @@
"index": "0x0",
"validatorIndex": "0x0",
"address": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b",
"amount": "0x1"
"amount": "0x3"
},
{
"index": "0x1",
"validatorIndex": "0x0",
"validatorIndex": "0x1",
"address": "0xfe3b557e8fb62b89f4916b721be55ceb828dbd73",
"amount": "0x2"
"amount": "0x4"
}
],
"blockNumber": "0x2",
"blockHash": "0x5dec96e86ccd61362959cc73a0992b033fcde6f01d7467393363538c74c5260d",
"blockHash": "0x4a7b0e390d320a07150daa9c3b9dddfc6b7f6ca7d7a2971ef2f53ac1f29f24dc",
"receiptsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421"
},
"blockValue": "0x0"

@ -24,13 +24,13 @@
},
{
"index": "0x1",
"validatorIndex": "0x0",
"validatorIndex": "0x1",
"address": "0xfe3b557e8fb62b89f4916b721be55ceb828dbd73",
"amount": "0x2"
}
],
"blockNumber": "0x2",
"blockHash": "0x5dec96e86ccd61362959cc73a0992b033fcde6f01d7467393363538c74c5260d",
"blockHash": "0x4f88d512a0045bc6d447ba74a18eac0ed2ebb8d9faca325f5f55b2ca84be0705",
"receiptsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421"
}
],
@ -41,7 +41,7 @@
"id": 67,
"result": {
"status": "VALID",
"latestValidHash": "0x5dec96e86ccd61362959cc73a0992b033fcde6f01d7467393363538c74c5260d",
"latestValidHash": "0x4f88d512a0045bc6d447ba74a18eac0ed2ebb8d9faca325f5f55b2ca84be0705",
"validationError": null
}
},

@ -1,28 +0,0 @@
{
"request": {
"jsonrpc": "2.0",
"method": "engine_forkchoiceUpdatedV2",
"params": [
{
"headBlockHash": "0x5dec96e86ccd61362959cc73a0992b033fcde6f01d7467393363538c74c5260d",
"safeBlockHash": "0x5dec96e86ccd61362959cc73a0992b033fcde6f01d7467393363538c74c5260d",
"finalizedBlockHash": "0x5dec96e86ccd61362959cc73a0992b033fcde6f01d7467393363538c74c5260d"
},
null
],
"id": 67
},
"response": {
"jsonrpc": "2.0",
"id": 67,
"result": {
"payloadStatus": {
"status": "VALID",
"latestValidHash": "0x5dec96e86ccd61362959cc73a0992b033fcde6f01d7467393363538c74c5260d",
"validationError": null
},
"payloadId": null
}
},
"statusCode": 200
}

@ -0,0 +1,28 @@
{
"request": {
"jsonrpc": "2.0",
"method": "engine_forkchoiceUpdatedV2",
"params": [
{
"headBlockHash": "0x4f88d512a0045bc6d447ba74a18eac0ed2ebb8d9faca325f5f55b2ca84be0705",
"safeBlockHash": "0x4f88d512a0045bc6d447ba74a18eac0ed2ebb8d9faca325f5f55b2ca84be0705",
"finalizedBlockHash": "0x4f88d512a0045bc6d447ba74a18eac0ed2ebb8d9faca325f5f55b2ca84be0705"
},
null
],
"id": 67
},
"response": {
"jsonrpc": "2.0",
"id": 67,
"result": {
"payloadStatus": {
"status": "VALID",
"latestValidHash": "0x4f88d512a0045bc6d447ba74a18eac0ed2ebb8d9faca325f5f55b2ca84be0705",
"validationError": null
},
"payloadId": null
}
},
"statusCode": 200
}

@ -252,7 +252,7 @@ public class MergeCoordinator implements MergeMiningCoordinator, BadChainListene
final PayloadIdentifier payloadIdentifier =
PayloadIdentifier.forPayloadParams(
parentHeader.getBlockHash(), timestamp, prevRandao, feeRecipient);
parentHeader.getBlockHash(), timestamp, prevRandao, feeRecipient, withdrawals);
if (blockCreationTask.containsKey(payloadIdentifier)) {
LOG.debug(

@ -17,8 +17,12 @@ package org.hyperledger.besu.consensus.merge.blockcreation;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.datatypes.Quantity;
import org.hyperledger.besu.ethereum.core.Withdrawal;
import java.math.BigInteger;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonValue;
@ -50,24 +54,37 @@ public class PayloadIdentifier implements Quantity {
}
/**
* Create payload identifier for payload params.
* Create payload identifier for payload params. This is a deterministic hash of all payload
* parameters that aims to avoid collisions
*
* @param parentHash the parent hash
* @param timestamp the timestamp
* @param prevRandao the prev randao
* @param feeRecipient the fee recipient
* @param withdrawals the withdrawals
* @return the payload identifier
*/
public static PayloadIdentifier forPayloadParams(
final Hash parentHash,
final Long timestamp,
final Bytes32 prevRandao,
final Address feeRecipient) {
final Address feeRecipient,
final Optional<List<Withdrawal>> withdrawals) {
return new PayloadIdentifier(
timestamp
^ ((long) parentHash.toHexString().hashCode()) << 8
^ ((long) prevRandao.toHexString().hashCode()) << 16
^ ((long) feeRecipient.toHexString().hashCode()) << 24);
^ ((long) feeRecipient.toHexString().hashCode()) << 24
^ (long)
withdrawals
.map(
ws ->
ws.stream()
.sorted(Comparator.comparing(Withdrawal::getIndex))
.map(Withdrawal::hashCode)
.reduce(1, (a, b) -> a ^ (b * 31)))
.orElse(0));
}
@Override

@ -14,14 +14,19 @@
*/
package org.hyperledger.besu.consensus.merge.blockcreation;
import static java.util.Collections.emptyList;
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.GWei;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.ethereum.core.Withdrawal;
import java.util.List;
import java.util.Optional;
import org.apache.tuweni.bytes.Bytes32;
import org.apache.tuweni.units.bigints.UInt64;
import org.junit.Test;
public class PayloadIdentifierTest {
@ -43,8 +48,92 @@ public class PayloadIdentifierTest {
public void conversionCoverage() {
var idTest =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, Bytes32.random(), Address.fromHexString("0x42"));
Hash.ZERO, 1337L, Bytes32.random(), Address.fromHexString("0x42"), Optional.empty());
assertThat(new PayloadIdentifier(idTest.getAsBigInteger().longValue())).isEqualTo(idTest);
assertThat(new PayloadIdentifier(idTest.getAsBigInteger().longValue())).isEqualTo(idTest);
}
@Test
public void differentWithdrawalAmountsYieldDifferentHash() {
final List<Withdrawal> withdrawals1 =
List.of(
new Withdrawal(
UInt64.valueOf(100),
UInt64.valueOf(1000),
Address.fromHexString("0x1"),
GWei.of(1)),
new Withdrawal(
UInt64.valueOf(200),
UInt64.valueOf(2000),
Address.fromHexString("0x2"),
GWei.of(2)));
final List<Withdrawal> withdrawals2 =
List.of(
new Withdrawal(
UInt64.valueOf(100),
UInt64.valueOf(1000),
Address.fromHexString("0x1"),
GWei.of(3)),
new Withdrawal(
UInt64.valueOf(200),
UInt64.valueOf(2000),
Address.fromHexString("0x2"),
GWei.of(4)));
final Bytes32 prevRandao = Bytes32.random();
var idForWithdrawals1 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, prevRandao, Address.fromHexString("0x42"), Optional.of(withdrawals1));
var idForWithdrawals2 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, prevRandao, Address.fromHexString("0x42"), Optional.of(withdrawals2));
assertThat(idForWithdrawals1).isNotEqualTo(idForWithdrawals2);
}
@Test
public void differentOrderedWithdrawalsYieldSameHash() {
final List<Withdrawal> withdrawals1 =
List.of(
new Withdrawal(
UInt64.valueOf(100),
UInt64.valueOf(1000),
Address.fromHexString("0x1"),
GWei.of(1)),
new Withdrawal(
UInt64.valueOf(200),
UInt64.valueOf(2000),
Address.fromHexString("0x2"),
GWei.of(2)));
final List<Withdrawal> withdrawals2 =
List.of(
new Withdrawal(
UInt64.valueOf(200),
UInt64.valueOf(2000),
Address.fromHexString("0x2"),
GWei.of(2)),
new Withdrawal(
UInt64.valueOf(100),
UInt64.valueOf(1000),
Address.fromHexString("0x1"),
GWei.of(1)));
final Bytes32 prevRandao = Bytes32.random();
var idForWithdrawals1 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, prevRandao, Address.fromHexString("0x42"), Optional.of(withdrawals1));
var idForWithdrawals2 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, prevRandao, Address.fromHexString("0x42"), Optional.of(withdrawals2));
assertThat(idForWithdrawals1).isEqualTo(idForWithdrawals2);
}
@Test
public void emptyOptionalAndEmptyListWithdrawalsYieldDifferentHash() {
final Bytes32 prevRandao = Bytes32.random();
var idForWithdrawals1 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, prevRandao, Address.fromHexString("0x42"), Optional.empty());
var idForWithdrawals2 =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, prevRandao, Address.fromHexString("0x42"), Optional.of(emptyList()));
assertThat(idForWithdrawals1).isNotEqualTo(idForWithdrawals2);
}
}

@ -51,6 +51,7 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.EngineUpdateFo
import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
import org.hyperledger.besu.ethereum.core.Withdrawal;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
import org.hyperledger.besu.ethereum.mainnet.TimestampSchedule;
import org.hyperledger.besu.ethereum.mainnet.WithdrawalsValidator;
@ -248,7 +249,8 @@ public abstract class AbstractEngineForkchoiceUpdatedTest {
mockHeader.getHash(),
payloadParams.getTimestamp(),
payloadParams.getPrevRandao(),
payloadParams.getSuggestedFeeRecipient());
payloadParams.getSuggestedFeeRecipient(),
Optional.empty());
when(mergeCoordinator.preparePayload(
mockHeader,
@ -521,7 +523,8 @@ public abstract class AbstractEngineForkchoiceUpdatedTest {
mockHeader.getHash(),
payloadParams.getTimestamp(),
payloadParams.getPrevRandao(),
payloadParams.getSuggestedFeeRecipient());
payloadParams.getSuggestedFeeRecipient(),
Optional.empty());
when(mergeCoordinator.preparePayload(
mockHeader,
@ -576,7 +579,7 @@ public abstract class AbstractEngineForkchoiceUpdatedTest {
blockHeaderBuilder.number(10L).parentHash(mockParent.getHash()).buildHeader();
setupValidForkchoiceUpdate(mockHeader);
var withdrawals =
var withdrawalParameters =
List.of(
new WithdrawalParameter(
"0x1",
@ -589,24 +592,28 @@ public abstract class AbstractEngineForkchoiceUpdatedTest {
String.valueOf(System.currentTimeMillis()),
Bytes32.fromHexStringLenient("0xDEADBEEF").toHexString(),
Address.ECREC.toString(),
withdrawals);
withdrawalParameters);
final Optional<List<Withdrawal>> withdrawals =
Optional.of(
withdrawalParameters.stream()
.map(WithdrawalParameter::toWithdrawal)
.collect(Collectors.toList()));
var mockPayloadId =
PayloadIdentifier.forPayloadParams(
mockHeader.getHash(),
payloadParams.getTimestamp(),
payloadParams.getPrevRandao(),
payloadParams.getSuggestedFeeRecipient());
payloadParams.getSuggestedFeeRecipient(),
withdrawals);
when(mergeCoordinator.preparePayload(
mockHeader,
payloadParams.getTimestamp(),
payloadParams.getPrevRandao(),
Address.ECREC,
Optional.of(
withdrawals.stream()
.map(WithdrawalParameter::toWithdrawal)
.collect(Collectors.toList()))))
withdrawals))
.thenReturn(mockPayloadId);
assertSuccessWithPayloadForForkchoiceResult(
@ -638,7 +645,8 @@ public abstract class AbstractEngineForkchoiceUpdatedTest {
mockHeader.getHash(),
payloadParams.getTimestamp(),
payloadParams.getPrevRandao(),
payloadParams.getSuggestedFeeRecipient());
payloadParams.getSuggestedFeeRecipient(),
Optional.empty());
when(mergeCoordinator.preparePayload(
mockHeader,

@ -72,7 +72,7 @@ public abstract class AbstractEngineGetPayloadTest {
protected static final BlockResultFactory factory = new BlockResultFactory();
protected static final PayloadIdentifier mockPid =
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 1337L, Bytes32.random(), Address.fromHexString("0x42"));
Hash.ZERO, 1337L, Bytes32.random(), Address.fromHexString("0x42"), Optional.empty());
protected static final BlockHeader mockHeader =
new BlockHeaderTestFixture().prevRandao(Bytes32.random()).buildHeader();
private static final Block mockBlock =
@ -118,7 +118,7 @@ public abstract class AbstractEngineGetPayloadTest {
resp(
getMethodName(),
PayloadIdentifier.forPayloadParams(
Hash.ZERO, 0L, Bytes32.random(), Address.fromHexString("0x42")));
Hash.ZERO, 0L, Bytes32.random(), Address.fromHexString("0x42"), Optional.empty()));
assertThat(resp).isInstanceOf(JsonRpcErrorResponse.class);
verify(engineCallListener, times(1)).executionEngineCalled();
}

Loading…
Cancel
Save