-
Notifications
You must be signed in to change notification settings - Fork 130
[PAN-2829] Add opcode and precompiled support for versioning #1683
Conversation
Add the current contract's version to the message frame.
Different opcode sets for different account versions
Different precompile sets for different account versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems good to me :D
return registry; | ||
} | ||
|
||
public static PrecompileContractRegistry appendPrivacy( | ||
final PrecompileContractRegistry registry, | ||
public static PrecompileContractRegistry istanbul( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this is unused - should you add it to ProtocolSpecBuilder
or cut it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed adding it to the spec builder.
|
||
public OperationRegistry(final int numVersions) { | ||
Preconditions.checkArgument(numVersions >= 1); | ||
this.operations = new Operation[numVersions][NUM_OPERATIONS]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Optional) You might look into guava's RangeMap
, which could map a range of versions to a single operation, rather than creating distinct references for every op-version combo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a "hotspot" I opted for native arrays, on the presumption native arrays are faster than interfaces. When we get better performance monitoring in we can see how much the impact is.
@@ -39,6 +39,7 @@ | |||
long DEFAULT_NONCE = 0L; | |||
Wei DEFAULT_BALANCE = Wei.ZERO; | |||
int DEFAULT_VERSION = 0; | |||
int ISTANBUL_VERSION = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Optional) Not sure about storing ISTANBUL_VERSION
here - it feels very mainnet-evm-specific. Looks like you could get away with just keeping this as a local variable in MainnetProtocolSpecs
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
…switch # Conflicts: # ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolSpecs.java # ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/MessageFrame.java
PR description
Different opcode sets and precompiled contract sets for different account versions