Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

[PAN-2905] EIP-1706 - Disable SSTORE with gasleft lt call stipend #1706

Merged
merged 6 commits into from
Jul 15, 2019

Conversation

shemnon
Copy link
Contributor

@shemnon shemnon commented Jul 15, 2019

PR description

Add minimum remaining gas check to exceptional halt reasons. Configure it
to zero pre-istanbul and 2300 for istanbul.

…pend

Add minimum remaining gas check to exceptional halt reasons.  Configure it
to zero pre-istanbul and 2300 for istanbul.
@@ -27,8 +27,14 @@

public class SStoreOperation extends AbstractOperation {

public SStoreOperation(final GasCalculator gasCalculator) {
public static final Gas FRONTIER_MINIMUM = Gas.ZERO;
public static final Gas EIP_1706_MINIMUM = Gas.of(2300);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we follow the existing pattern, I think these gas values belong on the GasCalculator?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


private final Gas minumumGasRemaining;

public SStoreOperation(final GasCalculator gasCalculator, final Gas minumumGasRemaining) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(discussion) Since this new minimum gas check is backwards-compatible, should we just always apply it? Rather than having separate minima for frontier vs istanbul?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not backwards compatible. Over 100 reference tests have broken so far in my test run and the build is not done yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

169 to be exact. Across a broad set of tests.

# Conflicts:
#	ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetEvmRegistries.java
#	ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolSpecs.java
@shemnon shemnon merged commit 1b8420b into PegaSysEng:master Jul 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants