refactor(core): replace ResourceBounds map with a fixed-field struct#3819
Open
Ehsan-saradar wants to merge 2 commits into
NethermindEth:mainNethermindEth/juno:mainfrom
Ehsan-saradar:refactor/3742-resource-bounds-structEhsan-saradar/juno:refactor/3742-resource-bounds-structCopy head branch name to clipboard
Open
refactor(core): replace ResourceBounds map with a fixed-field struct#3819Ehsan-saradar wants to merge 2 commits intoNethermindEth:mainNethermindEth/juno:mainfrom Ehsan-saradar:refactor/3742-resource-bounds-structEhsan-saradar/juno:refactor/3742-resource-bounds-structCopy head branch name to clipboard
Ehsan-saradar wants to merge 2 commits into
NethermindEth:mainNethermindEth/juno:mainfrom
Ehsan-saradar:refactor/3742-resource-bounds-structEhsan-saradar/juno:refactor/3742-resource-bounds-structCopy head branch name to clipboard
Conversation
Ehsan-saradar
force-pushed
the
refactor/3742-resource-bounds-struct
branch
from
July 16, 2026 07:00
3073074 to
ce1cc64
Compare
Swap map[Resource]ResourceBounds on the v3 transaction types for a ResourceBoundsMap struct with explicit L1Gas/L2Gas/L1DataGas fields, which is clearer and avoids a map allocation per transaction. To keep already-stored transactions readable without a DB migration, the type implements MarshalCBOR/UnmarshalCBOR that reproduce the exact legacy map[Resource]ResourceBounds wire format. l1_data_gas still only enters the transaction hash when present (non-nil MaxPricePerUnit), matching the old map-key check, so historical tx hashes are unchanged.
Replace *map[Resource]ResourceBounds on the feeder starknet.Transaction with a ResourceBoundsMap struct. JSON tags keep the uppercase L1_GAS/L2_GAS/L1_DATA_GAS feeder keys, and omitzero preserves the pre-0.13.4 shape where l1_data_gas is absent, so both add_transaction requests and decoded feeder responses round-trip unchanged. sn2core and the rpc feeder adapters consume the struct directly instead of iterating a map.
Ehsan-saradar
force-pushed
the
refactor/3742-resource-bounds-struct
branch
from
July 25, 2026 05:39
ce1cc64 to
16f2075
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors v3 transaction resource-bounds representation in both core and starknet from map[Resource]ResourceBounds to fixed-field structs (L1Gas / L2Gas / L1DataGas) to improve clarity and remove per-transaction map allocations, while preserving legacy encodings and hash behavior for backward compatibility.
Changes:
- Introduces
core.ResourceBoundsMapwith CBOR marshal/unmarshal that preserves the legacy on-diskmap[Resource]ResourceBoundsencoding, plus tests that pin byte-for-byte compatibility and hashing presence rules. - Introduces
starknet.ResourceBoundsMapfor feeder JSON with uppercase keys and omission behavior for absentl1_data_gas, plus JSON round-trip tests. - Updates adapters/RPC/VM/genesis/test utilities to use the new struct-based bounds representation end-to-end.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vm/transaction.go | Adapts core.ResourceBoundsMap into the VM payload map, conditionally including l1_data_gas. |
| starknet/transaction.go | Adds feeder-facing ResourceBoundsMap struct and updates Transaction.ResourceBounds type accordingly. |
| starknet/resource_bounds_test.go | Adds JSON round-trip tests to preserve feeder wire format, including pre-0.13.4 omission behavior. |
| rpc/v9/transaction.go | Migrates v9 RPC adaptation and feeder conversion paths to the new bounds struct types. |
| rpc/v9/transaction_test.go | Updates v9 transaction tests to construct core.ResourceBoundsMap literals. |
| rpc/v9/block_test.go | Updates v9 block tests to reference fixed fields instead of map indexing. |
| rpc/v8/transaction.go | Migrates v8 RPC adaptation and feeder conversion paths to the new bounds struct types. |
| rpc/v8/transaction_test.go | Updates v8 transaction tests to construct core.ResourceBoundsMap literals. |
| rpc/v8/subscriptions_test.go | Updates v8 subscription tests to use core.ResourceBoundsMap. |
| rpc/v8/block_test.go | Updates v8 block tests to reference fixed fields instead of map indexing. |
| rpc/v10/transaction_test.go | Updates v10 tests to construct core.ResourceBoundsMap literals. |
| rpc/v10/subscriptions_test.go | Updates v10 subscription tests to use core.ResourceBoundsMap. |
| rpc/v10/block_test.go | Updates v10 block tests to reference fixed fields instead of map indexing. |
| rpc/v10/adapt_transaction.go | Updates v10 core↔RPC↔feeder adaptation to use struct resource bounds. |
| genesis/genesis.go | Updates genesis transaction adaptation to produce core.ResourceBoundsMap. |
| core/transaction.go | Introduces core.ResourceBoundsMap, preserves CBOR wire format, and updates hashing to use fixed fields with presence semantics. |
| core/resource_bounds_test.go | Adds CBOR golden compatibility tests and hash presence-guard tests for l1_data_gas. |
| consensus/proposer/proposer_test.go | Updates proposer test transaction construction to use core.ResourceBoundsMap. |
| consensus/p2p/validator/transition_test.go | Updates validator transition tests to use core.ResourceBoundsMap. |
| clients/feeder/feeder_test.go | Updates feeder client tests to expect *starknet.ResourceBoundsMap instead of a map pointer. |
| adapters/testutils/transaction_test_utils.go | Updates test utilities to return core.ResourceBoundsMap instead of a map. |
| adapters/testutils/synctransaction_test_utils.go | Updates sync transaction builders to use zero-valued core.ResourceBoundsMap{} for pre-v3 transactions. |
| adapters/sn2core/sn2core.go | Updates feeder→core adaptation to build core.ResourceBoundsMap and preserve absence semantics. |
| adapters/sn2core/sn2core_test.go | Updates adapter tests to use core.ResourceBoundsMap. |
| adapters/p2p2core/transaction.go | Updates p2p→core transaction adaptation to populate core.ResourceBoundsMap. |
| adapters/core2p2p/transaction.go | Updates core→p2p transaction adaptation to read from core.ResourceBoundsMap fields. |
| .golangci.yaml | Excludes core.ResourceBounds / core.ResourceBoundsMap from exhaustruct checks due to meaningful zero values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Closes #3742
Both
coreandstarknetresource bounds were modeled asmap[Resource]ResourceBounds. Since the keys are effectively fixed, this replaces them with a struct with explicitL1Gas/L2Gas/L1DataGasfields — clearer, and it drops a map allocation per transaction.Approach
core.ResourceBoundsMapstruct (L1Gas/L2Gas/L1DataGas) replacing themap[Resource]ResourceBoundsfield on the v3 transaction types, and the same for the feederstarknettype.MarshalCBOR/UnmarshalCBORthat keep the exact legacymap[Resource]ResourceBoundswire format. A golden test pins byte-for-byte equality across the three historical shapes — all three resources, the pre-0.13.4 two-resource form (nol1_data_gas), and the nil map on v0–v2 — so already-stored bytes still decode into the struct.l1_data_gasonly enters the tip/resources hash when it's actually set (non-nilMaxPricePerUnit), which reproduces the old map-key check, so pre-0.13.4 tx hashes stay identical.starknetstruct keeps the uppercaseL1_GAS/L2_GAS/L1_DATA_GASkeys and usesomitzero, so an absentl1_data_gasround-trips both ways.vmresource-bounds map as-is — it's the marshal-only Blockifier payload (with the shortL1_DATAkey), out of scope for this one.Testing
make lint— passcore,starknet,adapters/...,rpc/...,vm,genesis,consensus,clients/feeder) — pass, including the new backward-compat golden tests incore/resource_bounds_test.goandstarknet/resource_bounds_test.go.