feat(rate-limit): public From<&RateLimits> for OpcodeRateLimits - #7
Merged
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
The re-key was private, so Default was the only way to build the type and a consumer could not exercise it against a cheap table -- any test was forced to couple itself to upstream chia's exact numbers. Exposing the conversion rather than the fields keeps the numbers DERIVED: a caller picks the source table, never an individual limit, so the second set of drifting numbers this type exists to prevent stays prevented. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d
commented
Aug 11, 2026
MichaelTaylor3d
left a comment
Contributor
Author
There was a problem hiding this comment.
PASS (correctness gate) — head e4f180c696d972c2b9a0d12370b6a5acb2a6eede.
Verified independently, in a throwaway worktree (no shared checkout mutated):
Defaultunchanged — falsifiability PROVEN, not assumed. I reroutedDefaultto a table withHandshakeatfrequency = 1.0and re-ran:default_still_derives_from_the_upstream_tableFAILS (left: [Admitted, Deferred, Deferred]vsright: [Admitted, Admitted, Admitted]) whilea_caller_supplied_table_governs_the_limiterstays green. The second assertion (via_default[2] == Admitted) is the anti-vacuity guard that stops the probe degenerating intoDefault-vs-Default. Mutation reverted; worktree clean.- Re-key exists exactly once —
from_chiais gone crate-wide; the body lives only inimpl From<&RateLimits>(src/rate_limit.rs:59). - No fields made public, no field-wise constructor —
OpcodeRateLimits' five fields remain private (src/rate_limit.rs:38-44); the only public constructors areDefaultand the newFrom. Derivation preserved. &*V2_RATE_LIMITScorrectly derefs theLazy;Fromtakes a borrow.- Footgun documented — the impl doc-comment names the cross-version re-key hazard explicitly (shifted wire bytes → silent fall-through to
default_settings), and points atcrate::RateLimits, whichsrc/lib.rs:67-70does re-export alongsideV2_RATE_LIMITS, andProtocolMessageTypesatsrc/lib.rs:61. The advice is actionable. - Additive only — nothing removed or resignatured;
0.4.0 → 0.5.0is the right call for a semver-incompatible 0.x minor. - Gates —
cargo test --lib70/70 locally; all 8 required checks green incl.Coverage (>=80% lines). Zero review threads open.
Non-gating nit, resolved by me, no action needed: SPEC.md:366 writes Default` is defined as `From<&V2_RATE_LIMITS> — V2_RATE_LIMITS is a value, not a type, so the notation is loose; From<&RateLimits> applied to V2_RATE_LIMITS is what is meant. Not worth a round-trip.
Nothing handed to @copilot. Orchestrator owns undraft + merge.
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.
DO NOT MERGE — gate round pending
Closes the untestability of
OpcodeRateLimitsfor consumers (dig_ecosystem#2228, dig-gossip's port oftests/con_005_tests.rs).What changed
Purely additive. The re-key that derives the opcode table from a Chia
RateLimitswas private (fn from_chia,src/rate_limit.rs:48), soDefaultwas the only way to build the type — a consumer could not supply a small table with a lowfrequency/max_sizeand was forced to couple its tests to upstream Chia's exact numbers.from_chiais moved into that impl, not duplicated.Defaultnow readsSelf::from(&*V2_RATE_LIMITS)and is byte-identical to before.Deliberately NOT added: public fields and a field-wise constructor. The value of this type is that its numbers are derived from Chia's table rather than being a second set that can drift; a caller now chooses the source table, never an individual limit.
RateLimit,RateLimitsandV2_RATE_LIMITSwere already re-exported from the crate root (src/lib.rs:67), so a consumer can build a table without depending onchia-sdk-clientindependently — which is what keeps the module header's lockstep pin enforceable. The new impl's doc-comment states that a table keyed by a differentchia_protocolversion'sProtocolMessageTypesre-keys to shifted wire bytes and silently loosens every Chia opcode todefault_settings— the exact hazard that note describes.Blast radius checked
from_chiahad exactly ONE caller:impl Default, same file, line 69.OpcodeRateLimitsis constructed atsrc/link.rs:245(::default()) and in this module's tests; it is exported atsrc/lib.rs:125and named inSPEC.md:278. No consumer namesfrom_chia(it was private). Risk: LOW — additive, one intra-file call site rerouted. Blast radius established by ripgrep over the worktree plus a read of every call site; the gitnexus index was not used for this crate (§2.0 fallback, stated).Tests (TDD)
Both written first.
a_caller_supplied_table_governs_the_limiter— clonesV2_RATE_LIMITS, retunesHandshaketofrequency = 2.0, and assertsAdmitted, Admitted, Deferred. RED evidence: it did not compile before the impl existed (E0308atsrc/rate_limit.rs:299, the reflexiveFrom<Self>being the only candidate) — which is precisely the point, the property was not expressible under aDefault-only surface.default_still_derives_from_the_upstream_table— the regression guard for the delegation. Probes with the message the custom table classifies differently (a third handshake; upstream'sHandshakefrequency is 5) and assertsDefaultandfrom(&*V2_RATE_LIMITS)agree AND that the probe is discriminating.Fixture design: the payload is 16 bytes so no size cap can bind and the only budget that can refuse is
frequency— the axis the custom table moves. The assertion isDeferred, not merely "not admitted", so anUnsendablefrom a mis-sized fixture would fail rather than pass for the wrong reason. The verdict is observed throughadmit, so the test proves what a consumer can actually do rather than reading private fields.Load-bearing proof (committed first, mutation reverted after): rerouting
Defaultto a table withHandshakeatfrequency = 1.0makesdefault_still_derives_from_the_upstream_tableFAIL whilea_caller_supplied_table_governs_the_limiterstays green — the two tests are independent and the guard bites on exactly the defect it names.cargo test: 70 + 9 + 5 + 4 + 2 + 1 pass, 0 fail.cargo fmt --checkandcargo clippy --all-targets -D warningsclean.Version
0.4.0→0.5.0. Additive, but a 0.x MINOR is semver-incompatible, so consumers must re-pin — expected and intended.SPEC.md§7.4 and conformance row C13 updated in the same commit;CHANGELOG.mdleft to git-cliff.