Skip to content

Commit

Permalink
Pectra-devnet-0 discoveries (hyperledger#7121)
Browse files Browse the repository at this point in the history
- corrects gas checking in AUTH / AUTHCALL
- fixes bug in engine api v4
- corrects gas pricing in BLOCKHASH

---------

Signed-off-by: Lucas Saldanha <[email protected]>
Signed-off-by: Gabriel-Trintinalia <[email protected]>
Signed-off-by: Justin Florentine <[email protected]>
Signed-off-by: Jason Frame <[email protected]>
Signed-off-by: Danno Ferrin <[email protected]>
Signed-off-by: Danno Ferrin <[email protected]>
Co-authored-by: Lucas Saldanha <[email protected]>
Co-authored-by: Gabriel-Trintinalia <[email protected]>
Co-authored-by: Jason Frame <[email protected]>
Co-authored-by: Danno Ferrin <[email protected]>
Co-authored-by: Danno Ferrin <[email protected]>
Co-authored-by: Jason Frame <[email protected]>
  • Loading branch information
7 people authored May 17, 2024
1 parent 433ebd9 commit c90f18c
Show file tree
Hide file tree
Showing 12 changed files with 96 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class WithdrawalRequestParameter {
@JsonCreator
public WithdrawalRequestParameter(
@JsonProperty("sourceAddress") final String sourceAddress,
@JsonProperty("pubkey") final String validatorPubKey,
@JsonProperty("validatorPublicKey") final String validatorPubKey,
@JsonProperty("amount") final String amount) {
this.sourceAddress = sourceAddress;
this.validatorPubKey = validatorPubKey;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ public static ProtocolSpecBuilder atlantisDefinition(
messageCallProcessor,
true,
false,
false,
stackSizeLimit,
feeMarket,
CoinbaseFeePriceCalculator.frontier()))
Expand Down Expand Up @@ -372,6 +373,7 @@ public static ProtocolSpecBuilder spiralDefinition(
messageCallProcessor,
true,
true,
false,
stackSizeLimit,
feeMarket,
CoinbaseFeePriceCalculator.frontier()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ public static ProtocolSpecBuilder frontierDefinition(
messageCallProcessor,
false,
false,
false,
stackSizeLimit,
FeeMarket.legacy(),
CoinbaseFeePriceCalculator.frontier()))
Expand Down Expand Up @@ -297,6 +298,7 @@ public static ProtocolSpecBuilder spuriousDragonDefinition(
messageCallProcessor,
true,
false,
false,
stackSizeLimit,
feeMarket,
CoinbaseFeePriceCalculator.frontier()))
Expand Down Expand Up @@ -490,6 +492,7 @@ static ProtocolSpecBuilder londonDefinition(
messageCallProcessor,
true,
false,
false,
stackSizeLimit,
feeMarket,
CoinbaseFeePriceCalculator.eip1559()))
Expand Down Expand Up @@ -627,6 +630,7 @@ static ProtocolSpecBuilder shanghaiDefinition(
messageCallProcessor,
true,
true,
false,
stackSizeLimit,
feeMarket,
CoinbaseFeePriceCalculator.eip1559()))
Expand Down Expand Up @@ -703,6 +707,7 @@ static ProtocolSpecBuilder cancunDefinition(
messageCallProcessor,
true,
true,
false,
stackSizeLimit,
feeMarket,
CoinbaseFeePriceCalculator.eip1559()))
Expand Down Expand Up @@ -737,6 +742,7 @@ static ProtocolSpecBuilder pragueDefinition(
final MiningParameters miningParameters) {
final int contractSizeLimit =
configContractSizeLimit.orElse(SPURIOUS_DRAGON_CONTRACT_SIZE_LIMIT);
final int stackSizeLimit = configStackSizeLimit.orElse(MessageFrame.DEFAULT_MAX_STACK_SIZE);

final Address depositContractAddress =
genesisConfigOptions.getDepositContractAddress().orElse(DEFAULT_DEPOSIT_CONTRACT_ADDRESS);
Expand Down Expand Up @@ -766,6 +772,25 @@ static ProtocolSpecBuilder pragueDefinition(
MaxCodeSizeRule.of(contractSizeLimit), EOFValidationCodeRule.of(1, false)),
1,
SPURIOUS_DRAGON_FORCE_DELETE_WHEN_EMPTY_ADDRESSES))
// warm blockahsh contract
.transactionProcessorBuilder(
(gasCalculator,
feeMarket,
transactionValidator,
contractCreationProcessor,
messageCallProcessor) ->
new MainnetTransactionProcessor(
gasCalculator,
transactionValidator,
contractCreationProcessor,
messageCallProcessor,
true,
true,
true,
stackSizeLimit,
feeMarket,
CoinbaseFeePriceCalculator.eip1559()))

// use prague precompiled contracts
.precompileContractRegistryBuilder(MainnetPrecompiledContractRegistries::prague)
.requestsValidator(pragueRequestsValidator(depositContractAddress))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.core.feemarket.CoinbaseFeePriceCalculator;
import org.hyperledger.besu.ethereum.mainnet.blockhash.PragueBlockHashProcessor;
import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket;
import org.hyperledger.besu.ethereum.privacy.storage.PrivateMetadataUpdater;
import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult;
Expand All @@ -52,6 +53,7 @@
import com.google.common.collect.Multimap;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
import org.apache.tuweni.units.bigints.UInt256;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -72,6 +74,7 @@ public class MainnetTransactionProcessor {
private final boolean clearEmptyAccounts;

protected final boolean warmCoinbase;
protected final boolean warmBlockhash;

protected final FeeMarket feeMarket;
private final CoinbaseFeePriceCalculator coinbaseFeePriceCalculator;
Expand All @@ -83,6 +86,7 @@ public MainnetTransactionProcessor(
final AbstractMessageProcessor messageCallProcessor,
final boolean clearEmptyAccounts,
final boolean warmCoinbase,
final boolean warmBlockhash,
final int maxStackSize,
final FeeMarket feeMarket,
final CoinbaseFeePriceCalculator coinbaseFeePriceCalculator) {
Expand All @@ -92,6 +96,7 @@ public MainnetTransactionProcessor(
this.messageCallProcessor = messageCallProcessor;
this.clearEmptyAccounts = clearEmptyAccounts;
this.warmCoinbase = warmCoinbase;
this.warmBlockhash = warmBlockhash;
this.maxStackSize = maxStackSize;
this.feeMarket = feeMarket;
this.coinbaseFeePriceCalculator = coinbaseFeePriceCalculator;
Expand Down Expand Up @@ -322,6 +327,15 @@ public TransactionProcessingResult processTransaction(
if (warmCoinbase) {
addressList.add(miningBeneficiary);
}
if (warmBlockhash) {
addressList.add(PragueBlockHashProcessor.HISTORY_STORAGE_ADDRESS);
storageList.putAll(
PragueBlockHashProcessor.HISTORY_STORAGE_ADDRESS,
List.of(
UInt256.valueOf(
(blockHeader.getNumber() - 1)
% PragueBlockHashProcessor.HISTORY_SERVE_WINDOW)));
}

final long intrinsicGas =
gasCalculator.transactionIntrinsicGasCost(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ public CachingBlockHashLookup(
@Override
public Hash apply(final MessageFrame frame, final Long blockNumber) {
long currentBlockNumber = frame.getBlockValues().getNumber();
if (currentBlockNumber <= blockNumber || currentBlockNumber - blockNumber > maxLookback) {
long lookback = currentBlockNumber - blockNumber;
if (currentBlockNumber <= blockNumber || lookback > maxLookback) {
return Hash.ZERO;
}
final Hash cachedHash = hashByNumber.get(blockNumber);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ MainnetTransactionProcessor createTransactionProcessor(final boolean warmCoinbas
messageCallProcessor,
false,
warmCoinbase,
false,
MAX_STACK_SIZE,
FeeMarket.legacy(),
CoinbaseFeePriceCalculator.frontier());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ public class FrontierGasCalculator implements GasCalculator {

private static final long BALANCE_OPERATION_GAS_COST = 20L;

private static final long BLOCKHASH_OPERATION_GAS_COST = 20L;
/** The constant BLOCKHASH_OPERATION_GAS_COST = 20gwei. */
public static final long BLOCKHASH_OPERATION_GAS_COST = 20L;

private static final long EXP_OPERATION_BASE_GAS_COST = 10L;

Expand Down Expand Up @@ -391,7 +392,7 @@ public long getBalanceOperationGasCost() {
}

@Override
public long getBlockHashOperationGasCost() {
public long getBlockHashOperationGasCost(final MessageFrame frame) {
return BLOCKHASH_OPERATION_GAS_COST;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,10 @@ default long authCallOperationGasCost(
/**
* Returns the cost for executing a {@link BlockHashOperation}.
*
* @param frame The current frame
* @return the cost for executing the block hash operation
*/
long getBlockHashOperationGasCost();
long getBlockHashOperationGasCost(MessageFrame frame);

/**
* Returns the cost for executing a {@link ExpOperation}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import org.hyperledger.besu.evm.account.Account;
import org.hyperledger.besu.evm.frame.MessageFrame;

import org.apache.tuweni.units.bigints.UInt256;

/**
* Gas Calculator for Prague
*
Expand All @@ -32,8 +34,8 @@
* </UL>
*/
public class PragueGasCalculator extends CancunGasCalculator {
private final int AUTH_OP_FIXED_FEE = 3100;
private final long AUTH_CALL_VALUE_TRANSFER_GAS_COST = 6700;
private static final int AUTH_OP_FIXED_FEE = 3100;
private static final long AUTH_CALL_VALUE_TRANSFER_GAS_COST = 6700;

/** Instantiates a new Prague Gas Calculator. */
public PragueGasCalculator() {
Expand Down Expand Up @@ -99,4 +101,24 @@ public long authCallOperationGasCost(

return cost;
}

/** Address of the contract historic block hashes are stored in. */
public static final Address HISTORY_STORAGE_ADDRESS =
Address.fromHexString("0x25a219378dad9b3503c8268c9ca836a52427a4fb");

/** The HISTORY_SERVE_WINDOW */
public static final long HISTORY_SERVE_WINDOW = 8192;

@Override
public long getBlockHashOperationGasCost(final MessageFrame frame) {
if (frame == null) {
return super.getBlockHashOperationGasCost(null);
} else {
UInt256 slot = UInt256.valueOf(frame.getBlockValues().getNumber() % HISTORY_SERVE_WINDOW);
return BLOCKHASH_OPERATION_GAS_COST
+ (frame.warmUpStorage(HISTORY_STORAGE_ADDRESS, slot)
? getWarmStorageReadCost()
: getColdSloadCost());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.evm.EVM;
import org.hyperledger.besu.evm.account.Account;
import org.hyperledger.besu.evm.frame.ExceptionalHaltReason;
import org.hyperledger.besu.evm.frame.MessageFrame;
import org.hyperledger.besu.evm.gascalculator.GasCalculator;
import org.hyperledger.besu.evm.internal.Words;
Expand Down Expand Up @@ -62,14 +64,24 @@ public OperationResult execute(final MessageFrame frame, final EVM evm) {
long offset = clampedToLong(frame.getStackItem(1));
long length = clampedToLong(frame.getStackItem(2));

final long gasCost =
super.gasCalculator().authOperationGasCost(frame, offset, length, authority);
if (frame.getRemainingGas() < gasCost) {
return new OperationResult(gasCost, ExceptionalHaltReason.INSUFFICIENT_GAS);
}

byte yParity = frame.readMemory(offset, 1).get(0);
Bytes32 r = Bytes32.wrap(frame.readMemory(offset + 1, 32));
Bytes32 s = Bytes32.wrap(frame.readMemory(offset + 33, 32));
Bytes32 commit = Bytes32.wrap(frame.readMemory(offset + 65, 32));
Bytes32 invoker = Bytes32.leftPad(frame.getContractAddress());
// TODO add test for getting sender nonce when account does not exist
Bytes32 senderNonce =
Bytes32.leftPad(
Bytes.ofUnsignedLong(frame.getWorldUpdater().getAccount(authority).getNonce()));
Bytes.ofUnsignedLong(
Optional.ofNullable(frame.getWorldUpdater().getAccount(authority))
.map(Account::getNonce)
.orElse(0L)));
if (evm.getChainId().isEmpty()) {
frame.pushStackItem(UInt256.ZERO);
LOG.error("ChainId is not set");
Expand All @@ -79,9 +91,6 @@ public OperationResult execute(final MessageFrame frame, final EVM evm) {
Bytes.concatenate(
Bytes.ofUnsignedShort(MAGIC), evm.getChainId().get(), senderNonce, invoker, commit);
Bytes32 messageHash = Hash.keccak256(authPreImage);

final long gasCost =
super.gasCalculator().authOperationGasCost(frame, offset, length, authority);
Optional<SECPPublicKey> publicKey;
try {
SECPSignature signature =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import org.apache.tuweni.units.bigints.UInt256;

/** The Block hash operation. */
public class BlockHashOperation extends AbstractFixedCostOperation {
public class BlockHashOperation extends AbstractOperation {

/** Frontier maximum relative block delta */
public static final int MAX_RELATIVE_BLOCK = 256;
Expand All @@ -46,24 +46,25 @@ public interface BlockHashLookup extends BiFunction<MessageFrame, Long, Hash> {}
* @param gasCalculator the gas calculator
*/
public BlockHashOperation(final GasCalculator gasCalculator) {
super(0x40, "BLOCKHASH", 1, 1, gasCalculator, gasCalculator.getBlockHashOperationGasCost());
super(0x40, "BLOCKHASH", 1, 1, gasCalculator);
}

@Override
public OperationResult executeFixedCostOperation(final MessageFrame frame, final EVM evm) {
public OperationResult execute(MessageFrame frame, EVM evm) {
final Bytes blockArg = frame.popStackItem().trimLeadingZeros();

// Short-circuit if value exceeds long
if (blockArg.size() > MAX_BLOCK_ARG_SIZE) {
frame.pushStackItem(UInt256.ZERO);
return successResponse;
return new OperationResult(gasCalculator().getBlockHashOperationGasCost(null), null);
}

final long soughtBlock = blockArg.toLong();
final BlockHashLookup blockHashLookup = frame.getBlockHashLookup();
final Hash blockHash = blockHashLookup.apply(frame, soughtBlock);
frame.pushStackItem(blockHash);

return successResponse;
return new OperationResult(
gasCalculator().getBlockHashOperationGasCost(Hash.ZERO.equals(blockHash) ? null : frame),
null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public void testAuthOperation() {
MutableAccount authingAccount = mock(MutableAccount.class);
when(authingAccount.getAddress()).thenReturn(authingAddress);
when(authingAccount.getNonce()).thenReturn(senderNonce);

when(frame.getRemainingGas()).thenReturn(1000000L);
WorldUpdater state = mock(WorldUpdater.class);

when(state.getAccount(authingAddress)).thenReturn(authingAccount);
Expand Down Expand Up @@ -121,6 +121,7 @@ public void testAuthOperationNegative() {
SECPSignature wrongSignature = algo.sign(messageHash, wrongKeys);

MessageFrame frame = mock(MessageFrame.class);
when(frame.getRemainingGas()).thenReturn(1000000L);
when(frame.getContractAddress()).thenReturn(invokerAddress);
MutableAccount authingAccount = mock(MutableAccount.class);
when(authingAccount.getAddress()).thenReturn(authingAddress);
Expand Down

0 comments on commit c90f18c

Please sign in to comment.