fix(gnovm): meter gas correctly for switch case

[davd-gzl]

fix:

• https://dashboard.hackenproof.com/manager/companies/newtendermint/gno-dot-land/reports/NEWTENDG-81
• https://dashboard.hackenproof.com/manager/companies/newtendermint/gno-dot-land/reports/NEWTENDG-184

Fix flat gas cost in doOpTypeSwitch by adding per-clause, per-case, and per-interface-method gas charges using existing CPU constants.

[Gno2D2]

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):

• IGNORE the bot requirements for this PR (force green CI check)

Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info)
🟢 Pending initial approval by a review team member, or review from tech-staff

☑️ Contributor Actions:

1. Fix any issues flagged by automated checks.
2. Follow the Contributor Checklist to ensure your PR is ready for review.
   • Add new tests, or document why they are unnecessary.
   • Provide clear examples/screenshots, if necessary.
   • Update documentation, if required.
   • Ensure no breaking changes, or include BREAKING CHANGE notes.
   • Link related issues/PRs, where applicable.

☑️ Reviewer Actions:

1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.

📚 Resources:

• Report a bug with the bot.
• View the bot’s configuration file.

Debug

Automated Checks

Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 The pull request was created from a fork (head branch repo: davd-gzl/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Pending initial approval by a review team member, or review from tech-staff

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)

Then

🟢 Requirement satisfied
└── 🟢 If
    ├── 🟢 Condition
    │   └── 🟢 Or
    │       ├── 🟢 User notJoon already reviewed PR 5217 with state APPROVED
    │       ├── 🟢 At least 1 user(s) of the team tech-staff reviewed pull request
    │       └── 🔴 This pull request is a draft
    └── 🟢 Then
        └── 🟢 Not (🔴 This label is applied to pull request: review/triage-pending)

Manual Checks

**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

• Any user with comment edit permission

[codecov]

Codecov Report

❌ Patch coverage is 83.78378% with 12 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
gnovm/pkg/gnolang/types.go	81.25%	7 Missing and 5 partials ⚠️

📢 Thoughts on this report? Let us know!

[mvallenet]

Should you add a filetest to compare gas usage depending on the size of switch case ?

[davd-gzl]

And I added the filetests: 9a38c56

[notJoon]

Could you resolve the conflict? thank you!

[jefft0]

This PR was approved by MikaelVallenet. Ready for core dev reiview.

[lbrown2007]

@omarsy @notJoon this PR "At least 1 approving review is required to merge this pull request." Could one of you review?

[thehowl]

A few comments to improve but overall looks good!

[thehowl]

The charge here is 4 × M, purely proportional to the method count. But each method check (findEmbeddedFieldType) also has a fixed cost that doesn't depend on M — mostly allocating a fresh seen map plus call overhead. Since we only bill the per-method part, that fixed part goes uncharged, and no single value of this constant can cover both a fixed and a per-method cost.

The benchmark data already in the repo shows the two parts. BenchmarkOpTypeAssert1_Interface_{1,10,100} (in gnovm/cmd/calibrate/op_bench_do_dedicated.txt, the same machine that produced the old fit: 348.9) measures roughly 197 / 1530 / 34000 ns, which works out to about 125 + 2.2×M ns per check (≈1 gas per ns). So 4×M covers the slope with headroom but charges nothing for the ~125 fixed part: we undercharge a lot at low method counts (~32× at M=1, ~3.7× at M=10), break even around M≈62, and slightly overcharge at M≥100.

To be clear, this isn't a reopened DoS — the MaxInterfaceMethods/MaxStructFields caps keep worst-case sustained amplification to ~2–4× once op and loop overhead are counted. It's more that we've lost accuracy in the common case compared to the old 349/method.

Fix is small: add a fixed base next to the slope, e.g. have perInterfaceMethodCheckCost return OpCPUIfaceProbeBase + OpCPUInterfaceMethodCheck*count with base ≈ 125. That matches all three measured points and only nudges the goldens.

[davd-gzl]

Fix: aba6dc6

[thehowl]

This test doesn't exercise the per-clause charging it describes: case int matches on the first clause, so matchLoop breaks immediately and the OpCPUSwitchClause/OpCPUSwitchClauseCase charges for the other 7 clauses are never applied. The +102 gas vs typeswitch_clauses_small.gno is just source-size overhead — the runtime scan gas is identical in both tests.

Verified by moving case int to the end: gas becomes 5285 vs the committed 3916, a delta of 1369 ≈ 7 × (87 + 109), which is exactly the scan charge this golden is meant to pin down. Suggest reordering so the matching case is last (and updating the golden to 5285) so the test actually locks the per-clause metering.

[davd-gzl]

Fix: aba6dc6

[thehowl]

This walk is itself unmetered and recomputed on every doOpTypeAssert1/2 execution (e.g. every iteration of a loop doing x.(I)): each call allocates a fresh seen map and can visit up to ~128 embedded fields of free work per op. For a 1-method interface miss against a struct with 128 embedded fields in a tight loop, that's roughly 2.4µs of real work for ~600 gas (~4× undercharge), dominated by this uncharged walk.

Suggestion: memoize the count on the type — there's precedent right below with the effectiveInterfaceMethods cache — or fold a charge for the walk into the op cost. Fine as a follow-up to this PR.

[davd-gzl]

Left as a follow-up, will do in a next PR!

[thehowl]

The *DeclaredType case undercounts when the base is an *InterfaceType — i.e. every named interface (error, io.Writer, fmt.Stringer, …). A named interface's methods live in its underlying *InterfaceType, not in DeclaredType.Methods (that slice holds concrete methods), so len(tt.Methods) is 0, the baseOf(tt).(*StructType) branch doesn't fire, and the function falls through to the n < 1 → return 1 clamp. It counts 1 regardless of the interface's real method count.

A bare named interface can't be ot directly (the assert handlers early-return on interface xt, and a type-switch xv.T is always concrete), but a concrete type that embeds a named interface hits this — the common type S struct { io.Writer } pattern. Verified with two equivalent 10-method structs in a case I type switch:

concrete type	count	perCheck (×4)	verify (10 probes)
struct{ Inner } (embeds 10-method struct)	10	40	400
struct{ W } (embeds 10-method named iface)	1	4	40

Both do the same findEmbeddedFieldType work (each probe walks into the embedded field and scans its method set), so the O(N×M) cost is identical, but the embedded-interface case is charged ~10× less (measured totals 5829 vs 6376 gas). The bare-*InterfaceType case counts correctly; the gap is specifically named interfaces.

Suggested fix:

case *DeclaredType:
    n = int64(len(tt.Methods))
    switch bt := baseOf(tt).(type) {
    case *StructType:
        n += countEmbeddedFieldMethods(bt, seen)
    case *InterfaceType:
        n += countTypeMethodsForGas(bt, seen)
    }

Note this compounds with the 4×M slope issue above — even once the interface base is counted, the missing per-probe intercept still under-weights small/mid method counts.

[davd-gzl]

Fix: aba6dc6