Skip to content

PLT-1024: generalize OperationMix to a named weighted-operation basket - #60

Open
bdchatham wants to merge 2 commits into
mainfrom
brandon2/plt-1024-named-operation-basket
Open

PLT-1024: generalize OperationMix to a named weighted-operation basket#60
bdchatham wants to merge 2 commits into
mainfrom
brandon2/plt-1024-named-operation-basket

Conversation

@bdchatham

@bdchatham bdchatham commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Generalizes OperationMix from three StorageRW-specific struct fields to a named weighted basket, so upcoming workloads can declare their own operations in a profile. The token basket has ~10; DeFi has 12.

Shape

Three types, one job each:

  • OperationMix = map[string]uint64 — the wire weights, keyed by name.
  • OperationSet — the operations a scenario supports, in draw order. StorageRWOperations declares rmw, read, write.
  • OperationPicker — one mix resolved against one set, built once per scenario.

Select returns a name; StorageRW switches on the same constants the set is declared from.

Determinism

Go map iteration order is unspecified, so a naive named-weight map silently loses the reproducibility the three-field version got free from its fixed comparison chain. Draw order comes from the set's declared slice, never from the mix.

The evidence it worked: TestStorageRWDrawOrderIsStable's golden values are unchanged, so the seeded sequence provably did not move. Mutating the picker to walk the map fails the dedicated order test 20/20; the golden alone catches it only ~11/20, which is why the 200-rebuild guard exists.

The golden fixture now decodes its mix from a JSON literal with alphabetical keys. A Go map literal in declared order iterates that way ~75% of the time, so the golden was majority-blind to the very mutation it names; alphabetical insertion inverts the dominant order and makes it deterministic.

Wire compatibility

{"read": 2, "write": 3, "rmw": 5} decodes identically into a map as into the struct, so every StorageRW profile keeps parsing and the FROZEN contract does not move. Scenario.Operations becomes a value, and absent → nil / {} → non-nil-len-0 still distinguishes the default from the error.

Preserved

An absent mix draws no randomness; a present-but-all-zero mix is a config error; weights past uint64 are a config error; an unknown name fails at load. profiles_test.go is unmodified.

Review fixes

  • Picker did not defend the overflow invariant Validate enforces. Measured with {rmw: 2^63, read: 2^63, write: 7}: Validate errors, but a picker on the same mix wraps total to 7 and draws rmw 10,000/10,000 — a workload nobody configured. The same loop already asserts the undeclared-name invariant; it now asserts both.
  • Corrected the StorageRW doc on read and contention. Investigation against sei-chain main established that Sei validates by comparing read sets to committed writes by value, and readAccumulator sits in both sets of every read. So reads do not conflict while a slot holds zero, and do once anything has written the keyspace. The limitation is real but conditional, and the doc now says which condition.

Breaking, wire unchanged

config.Operation removed; OpRmw/OpRead/OpWrite are strings; OperationMix is a map; Scenario.Operations is a value. Commit carries a BREAKING CHANGE trailer.

Verifier

gofmt, go vet, golangci-lint (pinned 2.12.2) 0 issues, go test -race ./... all pass.

For review

The declared order in StorageRWOperations is a one-way door — reordering or renaming changes what an old profile draws at the same seed. Recorded in config/doc.go's FROZEN section.

config now knows the name "storagerw", duplicating scenarios.StorageRW, because ValidateScenarios() stays no-arg. Review noted the repo already solves this shape with codegen for factory.go, and that generating config/scenario_operations.go the same way would remove the hand-maintained duplicate. Worth doing before the second and third baskets land; out of scope here.

bdchatham and others added 2 commits August 19, 2026 20:49
…basket

OperationMix weights operations by name. A scenario declares its own basket in a
profile, instead of a hardcoded mix in Go.

A scenario declares its operations as an ordered OperationSet. A profile weights
them with the same "operations" object. OperationSet.Picker resolves one mix into
one cumulative table at scenario construction. OperationPicker.Select then draws
from that table per transaction, at 8.5 ns and no allocation.

The declared order fixes the draw order. Go map iteration order is unspecified. A
draw that walked the weight map would give one sub-range of a draw to a different
operation on every process. storagerw declares rmw, read, write, in the order the
three-field comparison chain used, so the seeded draw sequence does not move.

Scenario.Validate rejects an operation the scenario does not declare, and names
both. It keeps the other rules. An absent mix is the default and draws no
randomness. An all-zero mix is an error. Weights that sum past uint64 are an
error.

The JSON wire form does not change. Every storagerw profile parses to the same
weights, and the frozen names in config/doc.go stay frozen.

BREAKING CHANGE: the Go API changes. This commit removes config.Operation, makes
OpRmw, OpRead and OpWrite strings, makes OperationMix a map, and makes
Scenario.Operations a value. The JSON wire form is unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s table

Validate rejects a mix whose weights sum past uint64, but Picker accumulated the
same weights without checking. Measured with {rmw: 2^63, read: 2^63, write: 7}:
Validate errors, while a picker built on that mix wraps its total to 7, leaves a
cumulative table of [2^63, 0, 7], and draws rmw 10000 times out of 10000 — a
workload nobody configured, arrived at silently because every individual weight
still looks sane.

The asymmetry was the tell: the same loop already asserts the undeclared-name
invariant with a panic rather than trusting the caller to have validated. It now
asserts both.

Also corrects what the StorageRW doc says about read and contention. It claimed
reads never sweep the key axis. Investigation against sei-chain main established
that Sei validates a transaction by comparing its read set to committed writes by
value, and readAccumulator sits in both the read and write set of every read. So
while a slot still holds zero the accumulator write is value-unchanged and reads
do not conflict; once anything has written the keyspace the accumulator changes
on every read and they do. Any mix containing rmw or write crosses that threshold
within a few blocks, so the limitation is real but conditional, and the doc now
says which condition.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bdchatham
bdchatham marked this pull request as ready for review August 20, 2026 04:25
@cursor

cursor Bot commented Aug 20, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches workload reproducibility and config validation on the hot transaction path, but behavior for default/absent mixes and storagerw draw order is heavily tested; main risk is the duplicated storagerw catalogue in config drifting from scenario implementations.

Overview
Replaces the fixed read/write/rmw struct with a scenario-specific operation basket so future workloads can declare their own operation names while keeping the same JSON shape (operations as name → weight).

OperationMix is now a map[string]uint64; OperationSet fixes draw order (rmw, read, write for storagerw) and OperationPicker precomputes cumulative weights once per scenario. Scenario.Validate resolves the scenario via scenarioOperations and rejects unknown names, all-zero mixes, and uint64 overflow. Draw order follows the set, not map iteration, so seeded replay for existing storagerw profiles stays unchanged (guarded by golden draw-order tests).

StorageRW builds a picker at construction and switches on string operation names instead of a removed config.Operation enum and pickOp. Docs expand the frozen wire contract and StorageRW read/contention behavior.

Reviewed by Cursor Bugbot for commit d15238c. Bugbot is set up for automated code reviews on this repo. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant