test(tbtc): add reservation proposal marshaling coverage - #4277
Draft
piotr-roslaniec wants to merge 2 commits into
Draft
test(tbtc): add reservation proposal marshaling coverage#4277piotr-roslaniec wants to merge 2 commits into
piotr-roslaniec wants to merge 2 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Closes gap-analysis Major row 1 and implementation-plan.md M1 row 3. ReservationAnchorProposal, ReservedRedemptionProposal, ReservationReanchorProposal, and ReservationDissolutionProposal previously used a JSON Marshal/Unmarshal placeholder, unlike every other CoordinationProposal type in this package (Heartbeat, DepositSweep, Redemption, MovingFunds, MovedFundsSweep), which all marshal via pkg/tbtc/gen/pb. Added the four missing message types to message.proto and regenerated message.pb.go (protoc 3.21.12 installed for this). Moved the four proposals' Marshal/Unmarshal from reservation.go's JSON stubs into marshaling.go, matching the existing proto-based implementations' structure and field-encoding conventions (big.Int fees via .Bytes()/SetBytes(), fixed-size hashes/pubkey-hashes via byte-slice copy with a length check). Preserved the original JSON stubs' validation intent under proto3's zero-value-is-absence semantics: a request nonce of 0, or empty fee/reservation-key/hash bytes, are rejected the same way an explicitly-missing JSON field was. The original '== nil' checks on *big.Int fields don't carry over as-is - SetBytes never returns nil - so they're now byte-length checks on the wire field instead, which is the pattern every other proto-based proposal in this file already uses. Testing: extended the existing table-driven TestCoordinationMessage_MarshalingRoundtrip with the four new types (exact field-for-field equality through the wire, matching the existing test's own precision, not just the fuzz-style tests already covering every sibling type) plus four new TestFuzzCoordinationMessage_MarshalingRoundtrip_With<X>Proposal crash-safety tests, matching the one-per-type convention. Rewrote the pre-existing TestReservationProposals_UnmarshalRejectsMissingIntegers (now TestReservationProposals_UnmarshalRejectsInvalidFields) to construct real protobuf payloads instead of JSON string literals, porting every original missing-field case plus two new structural cases (invalid hash/pubkey-hash length) that fall out of the new wire format. go test ./pkg/tbtc/...: 15/15 new/changed tests pass, full package suite passes (146s), -race clean (156s). gofmt/vet clean on all 6 changed files.
…g coverage - rename TestReservationProposals_UnmarshalRejectsMissingIntegers to ...RejectsInvalidFields, matching what the PR description already claimed - add rejection cases proving a zero *big.Int fee/key marshals to the same empty-bytes wire representation as an omitted field, exercised through each proposal's real Marshal() method - fix reservation fuzz test loops to match the sibling for-i convention
piotr-roslaniec
force-pushed
the
m1/reservation-protobuf-marshaling
branch
from
September 2, 2026 18:13
5c1f8f5 to
87c6a68
Compare
piotr-roslaniec
changed the title
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
1 participant
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.
Summary
Adds test coverage for the reservation
CoordinationProposalprotobufmarshaling (
ReservationAnchorProposal,ReservationReanchorProposal) thatswitched from a JSON placeholder to real protobuf via #4276 (stacked base
m1/reservation-readiness-fixes), matching every other proposal type inpkg/tbtc(Heartbeat, DepositSweep, Redemption, MovingFunds,MovedFundsSweep). The protobuf message definitions, generated bindings, and
Marshal/Unmarshalimplementations (including required-field validation)already exist on the base branch; this PR does not change that production
code.
Stacked on
m1/reservation-readiness-fixesper the delivery plan'sfollow-up-PR sequencing.
Change
pkg/tbtc/marshaling_test.go: extended the existing table-drivenTestCoordinationMessage_MarshalingRoundtripwith both reservationproposal types (exact field-for-field equality through the wire), plus two
new
TestFuzzCoordinationMessage_MarshalingRoundtrip_With<X>Proposalcrash-safety tests, one per type, matching the existing
one-per-sibling-type convention.
pkg/tbtc/reservation_test.go: renamedTestReservationProposals_UnmarshalRejectsMissingIntegerstoTestReservationProposals_UnmarshalRejectsInvalidFields, matching thebase branch's already-broader field-validation coverage. Added three new
cases proving that a legitimately-constructed proposal with a zero
*big.Intfee/key value is rejected as missing on unmarshal (thedocumented
.Bytes()empty-slice equivalence), exercised through eachproposal's real
Marshal()method rather than a hand-built protobufpayload.
Testing
go test ./pkg/tbtc/...: full package suite passes (362/362), zerofailures.
gofmt -l/go vet: clean on all changed files.Not in this PR
proposals are actually reachable in production - tracked separately.
test - both tracked as separate follow-up PRs per the implementation plan.
Marshal/Unmarshalimplementations, and their required-fieldvalidation) lives on the base branch via fix(spv): re-verify reservation action generation before SPV proof submission #4276, not in this PR's diff;
the milestone row should be closed against that PR, not this one.