feat(gno.land): meter type-check+preprocess gas at AddPackage and Run#5892
Draft
jaekwon wants to merge 4 commits into
Draft
feat(gno.land): meter type-check+preprocess gas at AddPackage and Run#5892jaekwon wants to merge 4 commits into
jaekwon wants to merge 4 commits into
Conversation
MsgAddPackage and MsgRun run two native passes over the submitted package -- go/types type-checking (all .gno files) and gnovm preprocessing (the prod subset) -- with zero gas charged for either: unmetered validator CPU, linear in the submitted source bytes. Add a governable vm param PreprocessGasPerByte (proto field 14, default 1250, measured ~1250 gas/byte on realistic example packages at 1 gas == 1ns on the reference machine; validated positive, capped at 100_000) and charge it per .gno source byte (prod + _test + _filetest) in AddPackage and Run, up front, so an oversized package is rejected by the gas meter before the native work runs. The byte base includes test files because the type-check pass checks them too; charging prod-only bytes would leave test-heavy packages unmetered. The non-zero default serializes into genesis vm params state, so the pinned genesis app hash moves (apphash_crossrealm38_test.go). Txtar fixtures take -gas-wanted bumps; exact gas pins are re-derived (addpkg_import_testdep_gas, gnokey_gasfee, restart_gas, maketx_run). The charge prices the typical case and is deliberately linear; it is not a worst-case bound for adversarial type-system inputs, and it does not yet price transitively re-type-checked dependency source (a planned follow-up adds a per-dep-byte term). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A vm params blob written before the field existed has no vm:p:preprocess_gas_per_byte key, so GetStruct left the field zero. That had two consequences on such legacy state: the type-check/ preprocess charge was silently disabled (0 gas per byte), and -- because WillSetParam re-validates the whole struct read via GetParams -- the first governance update of ANY other vm param panicked with "PreprocessGasPerByte must be positive" until the new key was set first. Zero is otherwise unrepresentable (Validate rejects it on every write path: SetParams, WillSetParam, ValidateGenesis), so a decoded zero can only mean pre-field state. Default it to preprocessGasPerByteDefault in GetParams: the charge stays active on legacy chains and unrelated param updates validate cleanly. State self-heals on the next SetParams or genesis export. NB the sibling IterNextCostFlat has the same latent trap from its own rollout (pre-field-13 state); left as-is since changing its legacy behavior (0 -> 1000 per iter step) is out of scope here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two review follow-ups on the mempackage-split branch: - Genesis/GetParams asymmetry: GetParams defaults a missing (zero) PreprocessGasPerByte to 1250 for legacy running state, but ValidateGenesis called Params.Validate() directly, which rejects zero. A relaunch genesis exported by a binary predating the field omits it (decodes as zero), so InitChain would panic — despite the branch's migration model being a genesis relaunch. Extract Params.applyLegacyDefaults and apply it in GetParams, ValidateGenesis (covering the standalone gnoland genesis-validate path too), and InitGenesis (so the stored value is the default, not zero). An explicitly out-of-range genesis value still fails. - Drop an allocation in the prod-less AddPackage guard: replace MPFProd.FilterMemPackage(memPkg).IsEmpty() (which allocated a full filtered copy) with hasProdGnoFile, scanning via MPFProd's own per-file FilterGno predicate — same source of truth as the storage split, no drift, no copy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):No automated checks match this pull request. ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Deploying a package (
MsgAddPackage) or running code (MsgRun) makes everyvalidator do two expensive steps before anything executes:
.gnofiles (Go's type checker), andToday neither step charges any gas. That means a large submission forces a lot
of real CPU work on every node for free — cheap for the sender, expensive for
the network, i.e. a denial-of-service hole.
This PR charges for that work, proportional to how much source was submitted:
PreprocessGasPerByte(default1250), charged per byte of
.gnosource.rejected by the gas meter instead of consuming free CPU first.
The charge counts all
.gnobytes (production and test files), becausethe type checker looks at the test files too — charging only production bytes
would leave part of the work unpaid.
Also included
stored settings were written before this parameter existed, the missing value
is treated as the default (1250) rather than zero — both while running and
when validating genesis. Without this, a missing value would silently turn the
charge off, or block unrelated settings changes.
longer makes a throwaway copy of the package just to test it.
Heads up: this changes the app hash and raises gas costs
the genesis app hash changes. Like PR1, shipping this needs a chain relaunch.
dozen test fixtures are re-pinned to the new, higher numbers, and the pinned
app hash is updated.
Testing
behavior, and genesis tolerating an older settings file.
gno.land/pkg/sdk/vm, the txtar integration suite,and the gnovm filetests.
Files changed
keeper.go(charge in deploy + run),params.go(new
PreprocessGasPerBytesetting + legacy default + validation),genesis.go(tolerate an older settings file),
params_test.gopb3_gen.go,vm.proto(the new setting's wire field)testdata/*.txtargas re-pins, plus the app-hash testTwo notes: