From a90ea056f19915a4eca787fc066c080e1789c14c Mon Sep 17 00:00:00 2001 From: Justin Florentine Date: Wed, 18 Oct 2023 18:54:39 -0400 Subject: [PATCH] Unsigned timestamps and blob gas used (#6046) * lots of places an unsigned timestamp is a problem * adds unchecked annotations to OptionalUnsignedLong rpc parameter type --------- Signed-off-by: Justin Florentine --- .../merge/blockcreation/MergeCoordinator.java | 3 ++- .../engine/AbstractEngineForkchoiceUpdated.java | 6 ++---- .../methods/engine/AbstractEngineNewPayload.java | 3 ++- .../methods/engine/EngineForkchoiceUpdatedV1.java | 6 +----- .../methods/engine/EngineForkchoiceUpdatedV2.java | 6 +----- .../methods/engine/EngineForkchoiceUpdatedV3.java | 6 +----- .../methods/engine/EngineNewPayloadV3.java | 3 ++- .../parameters/UnsignedLongParameter.java | 15 +++++++++------ .../besu/ethereum/core/BlockHeaderBuilder.java | 2 -- .../mainnet/ParentBeaconBlockRootHelper.java | 7 +++++-- .../ethereum/mainnet/ScheduledProtocolSpec.java | 2 +- .../IncrementalTimestampRule.java | 2 +- 12 files changed, 27 insertions(+), 34 deletions(-) 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 d7e2856941f..ffd57c4b27e 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 @@ -594,7 +594,8 @@ && isDescendantOf(newHead, blockchain.getChainHeadHeader())) { Optional parentOfNewHead = blockchain.getBlockHeader(newHead.getParentHash()); if (parentOfNewHead.isPresent() - && parentOfNewHead.get().getTimestamp() >= newHead.getTimestamp()) { + && Long.compareUnsigned(newHead.getTimestamp(), parentOfNewHead.get().getTimestamp()) + <= 0) { return ForkchoiceResult.withFailure( INVALID, "new head timestamp not greater than parent", latestValid); } diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java index 6f65dc5bd3d..b5c38c1e682 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java @@ -140,7 +140,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) .map(WithdrawalParameter::toWithdrawal) .collect(toList()))); Optional maybeError = - isPayloadAttributesValid(requestId, payloadAttributes, withdrawals); + isPayloadAttributesValid(requestId, payloadAttributes); if (maybeError.isPresent()) { LOG.atWarn() .setMessage("RpcError {}: {}") @@ -229,9 +229,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) } protected abstract Optional isPayloadAttributesValid( - final Object requestId, - final EnginePayloadAttributesParameter payloadAttributes, - final Optional> maybeWithdrawals); + final Object requestId, final EnginePayloadAttributesParameter payloadAttribute); protected Optional isPayloadAttributeRelevantToNewHead( final Object requestId, diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java index c8263450e72..a5b8acfc056 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java @@ -267,7 +267,8 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) } if (maybeParentHeader.isPresent() - && (blockParam.getTimestamp() <= maybeParentHeader.get().getTimestamp())) { + && (Long.compareUnsigned(maybeParentHeader.get().getTimestamp(), blockParam.getTimestamp()) + >= 0)) { return respondWithInvalid( reqId, blockParam, diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1.java index 24d63fbd434..6aa5f0964b2 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1.java @@ -20,10 +20,8 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EnginePayloadAttributesParameter; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; -import org.hyperledger.besu.ethereum.core.Withdrawal; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; -import java.util.List; import java.util.Optional; import io.vertx.core.Vertx; @@ -41,9 +39,7 @@ public EngineForkchoiceUpdatedV1( @Override protected Optional isPayloadAttributesValid( - final Object requestId, - final EnginePayloadAttributesParameter payloadAttributes, - final Optional> maybeWithdrawals) { + final Object requestId, final EnginePayloadAttributesParameter payloadAttributes) { return Optional.empty(); } diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java index c4daa688d47..b6406a3e662 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java @@ -20,10 +20,8 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EnginePayloadAttributesParameter; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; -import org.hyperledger.besu.ethereum.core.Withdrawal; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; -import java.util.List; import java.util.Optional; import io.vertx.core.Vertx; @@ -52,9 +50,7 @@ public String getName() { @Override protected Optional isPayloadAttributesValid( - final Object requestId, - final EnginePayloadAttributesParameter payloadAttributes, - final Optional> maybeWithdrawals) { + final Object requestId, final EnginePayloadAttributesParameter payloadAttributes) { if (payloadAttributes.getTimestamp() >= cancunTimestamp) { if (payloadAttributes.getParentBeaconBlockRoot() == null || payloadAttributes.getParentBeaconBlockRoot().isEmpty()) { diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV3.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV3.java index 5b33f653fb3..c070d220854 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV3.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV3.java @@ -21,12 +21,10 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EnginePayloadAttributesParameter; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; -import org.hyperledger.besu.ethereum.core.Withdrawal; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.mainnet.ScheduledProtocolSpec; import org.hyperledger.besu.ethereum.mainnet.ValidationResult; -import java.util.List; import java.util.Optional; import io.vertx.core.Vertx; @@ -91,9 +89,7 @@ protected ValidationResult validateForkSupported(final long blockT @Override protected Optional isPayloadAttributesValid( - final Object requestId, - final EnginePayloadAttributesParameter payloadAttributes, - final Optional> maybeWithdrawals) { + final Object requestId, final EnginePayloadAttributesParameter payloadAttributes) { if (payloadAttributes.getParentBeaconBlockRoot() == null) { LOG.error( "Parent beacon block root hash not present in payload attributes after cancun hardfork"); diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3.java index c7d8a10a4a1..dff7174b49d 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3.java @@ -71,7 +71,8 @@ protected ValidationResult validateParameters( @Override protected ValidationResult validateForkSupported(final long blockTimestamp) { if (protocolSchedule.isPresent()) { - if (cancun.isPresent() && blockTimestamp >= cancun.get().milestone()) { + if (cancun.isPresent() + && Long.compareUnsigned(blockTimestamp, cancun.get().milestone()) >= 0) { return ValidationResult.valid(); } else { return ValidationResult.invalid( diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/UnsignedLongParameter.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/UnsignedLongParameter.java index 039592d28c2..2e9ae8087ec 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/UnsignedLongParameter.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/UnsignedLongParameter.java @@ -17,25 +17,28 @@ import static com.google.common.base.Preconditions.checkArgument; import com.fasterxml.jackson.annotation.JsonCreator; +import org.checkerframework.checker.signedness.qual.Unsigned; public class UnsignedLongParameter { - private final long value; + @Unsigned private final long value; @JsonCreator public UnsignedLongParameter(final String value) { checkArgument(value != null); - this.value = Long.decode(value); - checkArgument(this.value >= 0); + if (value.startsWith("0x")) { + this.value = Long.parseUnsignedLong(value.substring(2), 16); + } else { + this.value = Long.parseUnsignedLong(value, 16); + } } @JsonCreator - public UnsignedLongParameter(final long value) { + public UnsignedLongParameter(final @Unsigned long value) { this.value = value; - checkArgument(this.value >= 0); } - public long getValue() { + public @Unsigned long getValue() { return value; } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/BlockHeaderBuilder.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/BlockHeaderBuilder.java index e218bf1d25b..fc38b663ea9 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/BlockHeaderBuilder.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/BlockHeaderBuilder.java @@ -238,7 +238,6 @@ private void validateProcessableBlockHeader() { checkState(this.difficulty != null, "Missing block difficulty"); checkState(this.number > -1L, "Missing block number"); checkState(this.gasLimit > -1L, "Missing gas limit"); - checkState(this.timestamp > -1L, "Missing timestamp"); } private void validateSealableBlockHeader() { @@ -360,7 +359,6 @@ public BlockHeaderBuilder gasUsed(final long gasUsed) { } public BlockHeaderBuilder timestamp(final long timestamp) { - checkArgument(timestamp >= 0); this.timestamp = timestamp; return this; } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ParentBeaconBlockRootHelper.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ParentBeaconBlockRootHelper.java index 80eb7a19e9f..2c863add026 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ParentBeaconBlockRootHelper.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ParentBeaconBlockRootHelper.java @@ -18,6 +18,8 @@ import org.hyperledger.besu.evm.account.MutableAccount; import org.hyperledger.besu.evm.worldstate.WorldUpdater; +import com.google.common.primitives.Longs; +import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; import org.apache.tuweni.units.bigints.UInt256; @@ -34,14 +36,15 @@ static void storeParentBeaconBlockRoot( /* see EIP-4788: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-4788.md */ - final long timestampReduced = timestamp % HISTORY_BUFFER_LENGTH; + final long timestampReduced = Long.remainderUnsigned(timestamp, HISTORY_BUFFER_LENGTH); final long timestampExtended = timestampReduced + HISTORY_BUFFER_LENGTH; final UInt256 timestampIndex = UInt256.valueOf(timestampReduced); final UInt256 rootIndex = UInt256.valueOf(timestampExtended); final MutableAccount account = worldUpdater.getOrCreate(BEACON_ROOTS_ADDRESS); - account.setStorageValue(timestampIndex, UInt256.valueOf(timestamp)); + account.setStorageValue( + timestampIndex, UInt256.fromBytes(Bytes.of(Longs.toByteArray(timestamp)))); account.setStorageValue(rootIndex, UInt256.fromBytes(root)); worldUpdater.commit(); } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ScheduledProtocolSpec.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ScheduledProtocolSpec.java index 012bb469c6a..d03a3fe90ef 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ScheduledProtocolSpec.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ScheduledProtocolSpec.java @@ -62,7 +62,7 @@ private TimestampProtocolSpec(final long timestamp, final ProtocolSpec protocolS @Override public boolean isOnOrAfterMilestoneBoundary(final ProcessableBlockHeader header) { - return header.getTimestamp() >= timestamp; + return Long.compareUnsigned(header.getTimestamp(), timestamp) >= 0; } @Override diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/IncrementalTimestampRule.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/IncrementalTimestampRule.java index bc29e0db9f2..72b5161a775 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/IncrementalTimestampRule.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/IncrementalTimestampRule.java @@ -28,6 +28,6 @@ public boolean validate( final long blockTimestamp = header.getTimestamp(); final long parentTimestamp = parent.getTimestamp(); - return blockTimestamp > parentTimestamp; + return Long.compareUnsigned(blockTimestamp, parentTimestamp) > 0; } }