Skip to content

Commit

Permalink
Unsigned timestamps and blob gas used (#6046)
Browse files Browse the repository at this point in the history
* lots of places an unsigned timestamp is a problem
* adds unchecked annotations to OptionalUnsignedLong rpc parameter type

---------

Signed-off-by: Justin Florentine <justin+github@florentine.us>
  • Loading branch information
jflo committed Oct 18, 2023
1 parent 03a8335 commit a90ea05
Show file tree
Hide file tree
Showing 12 changed files with 27 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,8 @@ && isDescendantOf(newHead, blockchain.getChainHeadHeader())) {

Optional<BlockHeader> 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
.map(WithdrawalParameter::toWithdrawal)
.collect(toList())));
Optional<JsonRpcErrorResponse> maybeError =
isPayloadAttributesValid(requestId, payloadAttributes, withdrawals);
isPayloadAttributesValid(requestId, payloadAttributes);
if (maybeError.isPresent()) {
LOG.atWarn()
.setMessage("RpcError {}: {}")
Expand Down Expand Up @@ -229,9 +229,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
}

protected abstract Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
final Object requestId,
final EnginePayloadAttributesParameter payloadAttributes,
final Optional<List<Withdrawal>> maybeWithdrawals);
final Object requestId, final EnginePayloadAttributesParameter payloadAttribute);

protected Optional<JsonRpcErrorResponse> isPayloadAttributeRelevantToNewHead(
final Object requestId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -41,9 +39,7 @@ public EngineForkchoiceUpdatedV1(

@Override
protected Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
final Object requestId,
final EnginePayloadAttributesParameter payloadAttributes,
final Optional<List<Withdrawal>> maybeWithdrawals) {
final Object requestId, final EnginePayloadAttributesParameter payloadAttributes) {
return Optional.empty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -52,9 +50,7 @@ public String getName() {

@Override
protected Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
final Object requestId,
final EnginePayloadAttributesParameter payloadAttributes,
final Optional<List<Withdrawal>> maybeWithdrawals) {
final Object requestId, final EnginePayloadAttributesParameter payloadAttributes) {
if (payloadAttributes.getTimestamp() >= cancunTimestamp) {
if (payloadAttributes.getParentBeaconBlockRoot() == null
|| payloadAttributes.getParentBeaconBlockRoot().isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -91,9 +89,7 @@ protected ValidationResult<RpcErrorType> validateForkSupported(final long blockT

@Override
protected Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
final Object requestId,
final EnginePayloadAttributesParameter payloadAttributes,
final Optional<List<Withdrawal>> 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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ protected ValidationResult<RpcErrorType> validateParameters(
@Override
protected ValidationResult<RpcErrorType> 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -360,7 +359,6 @@ public BlockHeaderBuilder gasUsed(final long gasUsed) {
}

public BlockHeaderBuilder timestamp(final long timestamp) {
checkArgument(timestamp >= 0);
this.timestamp = timestamp;
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

0 comments on commit a90ea05

Please sign in to comment.