Get execution version from snapshot instead of state #6867
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6867 +/- ##
==========================================
- Coverage 41.18% 41.16% -0.02%
==========================================
Files 2109 2111 +2
Lines 185660 185681 +21
==========================================
- Hits 76460 76438 -22
- Misses 102788 102832 +44
+ Partials 6412 6411 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
janezpodhostnik
force-pushed
the
janez/execution-version-from-snapshot
branch
3 times, most recently
from
fa1e7df to
168376a
Compare
janezpodhostnik
force-pushed
the
janez/execution-version-from-snapshot
branch
from
168376a to
33bde51
Compare
| ) | ||
|
|
||
| // protocolStateWrapper implements `EntropyProviderPerBlock` | ||
| var _ fvm.ProtocolStateSnapshotProvider = protocolStateWrapper{} |
There was a problem hiding this comment.
| var _ fvm.ProtocolStateSnapshotProvider = protocolStateWrapper{} | |
| var _ fvm.ProtocolStateSnapshotProvider = (*protocolStateWrapper)(nil) |
|
|
||
| // SnapshotSubset is a subset of the protocol state snapshot that is needed by the FVM | ||
| // for execution. | ||
| type SnapshotSubset interface { |
There was a problem hiding this comment.
Can we define it in state/protocol/snapshot.go? And maybe embed into Snapshot interface?
| return func(ctx Context) Context { | ||
| ctx.ReadVersionFromNodeVersionBeacon = enabled | ||
| // `protocol.Snapshot` implements `EntropyProvider` interface |
There was a problem hiding this comment.
| // `protocol.Snapshot` implements `EntropyProvider` interface | |
| // `protocol.SnapshotSubset` implements `EntropyProvider` interface |
| // However, at this stage, snapshot reference block should be known and the QC should also be known, | ||
| // so no error is expected in normal operations, as required by `EntropyProvider`. | ||
| fvm.WithEntropyProvider(e.protocolState.AtBlockID(blockId)), | ||
| fvm.WithProtocolStateSnapshot(e.protocolState.AtBlockID(blockHeader.ID())), |
There was a problem hiding this comment.
| fvm.WithProtocolStateSnapshot(e.protocolState.AtBlockID(blockHeader.ID())), | |
| fvm.WithProtocolStateSnapshot(e.protocolState.AtBlockID(blockID)), |
| // However, at this stage, snapshot reference block should be known and the QC should also be known, | ||
| // so no error is expected in normal operations, as required by `EntropyProvider`. | ||
| fvm.WithEntropyProvider(e.protocolState.AtBlockID(blockId)), | ||
| fvm.WithProtocolStateSnapshot(e.protocolState.AtBlockID(blockHeader.ID())), |
There was a problem hiding this comment.
| fvm.WithProtocolStateSnapshot(e.protocolState.AtBlockID(blockHeader.ID())), | |
| fvm.WithProtocolStateSnapshot(e.protocolState.AtBlockID(blockID)), |
| @@ -216,15 +211,6 @@ func (computer ExecutionParametersComputer) getExecutionParameters() ( | |||
| return overrides, err | |||
| } | |||
|
|
|||
| executionVersion, err := GetMinimumRequiredExecutionVersion(computer.log, computer.ctx, env) | |||
There was a problem hiding this comment.
Just to confirm, this is the key change to remove the dependency on the execution state in order to prevent program cache to invalidated, right?
I wonder why program cache would be invalidated if the operation is a read (reading the minimum required execution version), instead of a write.
No description provided.