test(reservations): M2 test-coverage backfill (7 of 8 items) - #4280
Draft
piotr-roslaniec wants to merge 3 commits into
Draft
test(reservations): M2 test-coverage backfill (7 of 8 items)#4280piotr-roslaniec wants to merge 3 commits into
piotr-roslaniec wants to merge 3 commits into
Conversation
Implementation-plan.md Milestone 2, all items except the one item (ValidateReservationAnchorProposal/ValidateReservationReanchorProposal tests) that needs simulated-backend test infrastructure this repository doesn't have - see docs/spec/reservations/m1-keep-core- readiness/01-gap-analysis.md's new Minor row for that finding, and the item-8 dedup row for the golden-value equivalence decision. pkg/tbtc/reservation_test.go: - TestAssembleReservationAnchorTransaction: happy-path output shape (1-in-1-out, deposit value minus fee, P2WPKH to the target wallet). Also doubles as the golden reference for pkg/tbtcpg's dedup test. - TestAssembleReservationReanchorTransaction: same shape assertion for the re-anchor sibling. pkg/chain/ethereum/tbtc_test.go: - TestConvertReservationParametersFromAbiType: full 10-tuple field mapping, every field set to a distinct non-zero value so a swapped or dropped field can't hide behind a shared zero default. - TestConvertReservationFromAbiType_DropsCumulativeReanchorFee: pins the intentional CumulativeReanchorFee omission and verifies every other field maps correctly around it. pkg/tbtcpg/reservation_acceptance_test.go: - TestReservationAcceptanceTask_BoundaryChecks: 6-case table (at-limit accepts / one-over rejects) for MaxReservationsPerWallet, ReservationMinAmount, and ReservationMaxTotalAmount - BoundedLookback only ever used these as fixture data, never at the actual boundary. - TestReservationAcceptanceTask_ReservationParametersFetchedLive: runs the same task twice against the same deposit, mutating ReservationMinAmount between calls - proves ReservationParameters() is fetched live per call, not cached on the task. pkg/tbtcpg/reservation_anchor_dedup_test.go (new file, package tbtcpg - internal, not tbtcpg_test - to reach the unexported function): - TestBuildReservationAnchorTransaction_MatchesPkgTbtcGoldenOutput: buildReservationAnchorTransaction (pkg/tbtcpg) and assembleReservationAnchorTransaction (pkg/tbtc) are independently maintained copies of the same logic, both unexported in different packages - Go's visibility rules make a single test calling both impossible without a production-code change. This test and pkg/tbtc's TestAssembleReservationAnchorTransaction instead pin the identical golden input/output values (deposit 100000, fee 1500, output 98500) in each package, catching either copy drifting from the other without eliminating the underlying duplication (real fix deferred, per decision this session). Testing: - go test ./pkg/tbtc/... ./pkg/tbtcpg/... ./pkg/chain/ethereum/...: 519/519 pass. - go build ./... && go test ./...: full repo, 49 packages, zero FAIL. - gofmt -l / go vet: clean on all 4 changed/new files.
|
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 |
…servation-test-coverage-backfill
…servation-test-coverage-backfill Reconciles this branch's M2 test-coverage backfill work with several independent changes on the base: dead-code removal, the conversion function rename, and a new net-of-fee minimum-amount gate. - Deleted pkg/tbtcpg/reservation_anchor_dedup_test.go entirely: its sole test pinned buildReservationAnchorTransaction (tbtcpg package) against a golden value shared with pkg/tbtc's sibling function, to catch drift between the two independently-maintained duplicate implementations. c48d17b deleted buildReservationAnchorTransaction from production code (reservation_acceptance.go now derives the reservation key from the anchor UTXO instead of reassembling the transaction) - the duplication this test existed to guard against no longer exists, so the guard is moot, not just broken. - pkg/tbtc/reservation_test.go: fixed two tests (TestAssembleReservationAnchorTransaction, TestAssembleReservationReanchorTransaction) left calling the pre-rename lowercase assembleReservation{Anchor,Reanchor}Transaction; both functions were exported and gained a required *ReservationAction parameter (fee-ceiling guard) upstream. Updated call sites and doc comments; both tests' own value remains real (happy-path output-shape coverage the nearby InputValidation test doesn't provide). - pkg/chain/ethereum/tbtc_test.go: this branch and the base independently added TestConvertReservationParametersFromAbiType with different fixture values (both equally rigorous); kept this branch's version (doc-linked to the gap-analysis finding it backfills) and the base's two genuinely new, non-overlapping tests (TestConvertReservationFromAbiType, TestConvertReservationActionFromAbiType). - pkg/tbtcpg/reservation_acceptance_test.go: same collision pattern - this branch's TestReservationAcceptanceTask_ReservationParametersFetchedLive and the base's TestReservationAcceptanceTask_GetWalletError are unrelated tests that landed at the same insertion point; kept both. Fixed three latent fixture gaps this merge surfaced in this branch's own ReservationParametersFetchedLive/BoundaryChecks tests (pre-dating this merge, never run against current production code until now): missing EndBlock on the DepositRevealedEventFilter, missing Vault on the DepositRevealedEvent (both added by c48d17b's EndBlock/vault- match hardening), and a missing fee-oracle rate (SetEstimateSatPerVByteFee) now required by dynamic fee estimation. Also corrected the 'exactly at minimum' boundary case's deposit amount: proposeReservationAcceptance now enforces the minimum against the *net-of-fee* anchor value, not the gross deposit amount, so the boundary must be reservationMinAmount + the fixture's deterministic 710-sat fee, not reservationMinAmount itself. Full repo: zero FAIL.
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.
Summary
Implements
implementation-plan.mdMilestone 2's test-coverage backfill:7 of the 8 listed items. Stacked on
m1/reservation-multisigner-integration-test(#4279), tip
cb4334627.The 8th item (
ValidateReservationAnchorProposal/ValidateReservationReanchorProposaltests) is explicitly deferred - itneeds
go-ethereumsimulated-backend test infrastructure that doesn'texist anywhere in
pkg/chain/ethereumtoday, well beyond the plan's0.5-day estimate. See
docs/spec/reservations/m1-keep-core-readiness/01-gap-analysis.md'snew Minor row for the full finding.
Change
pkg/tbtc/reservation_test.goTestAssembleReservationAnchorTransaction: happy-path output shape(1-in-1-out, deposit value minus fee, P2WPKH to the target wallet). Also
doubles as the golden reference for
pkg/tbtcpg's dedup test below.TestAssembleReservationReanchorTransaction: same shape assertion forthe re-anchor sibling.
pkg/chain/ethereum/tbtc_test.goTestConvertReservationParametersFromAbiType: full 10-tuple fieldmapping, every field a distinct non-zero value so a swapped or dropped
field can't hide behind a shared zero default.
TestConvertReservationFromAbiType_DropsCumulativeReanchorFee: pins theintentional
CumulativeReanchorFeeomission and verifies every otherfield maps correctly around it.
pkg/tbtcpg/reservation_acceptance_test.goTestReservationAcceptanceTask_BoundaryChecks: 6-case table (at-limitaccepts / one-over rejects) for
MaxReservationsPerWallet,ReservationMinAmount, andReservationMaxTotalAmount-TestReservationAcceptanceTask_BoundedLookbackonly ever used thesefields as fixture data, never at the actual boundary.
TestReservationAcceptanceTask_ReservationParametersFetchedLive: runsthe same task twice against the same deposit, mutating
ReservationMinAmountbetween calls - provesReservationParameters()is fetched live per call, not cached on the task.
pkg/tbtcpg/reservation_anchor_dedup_test.go(new file,package tbtcpg- internal, nottbtcpg_test- to reach the unexported function)TestBuildReservationAnchorTransaction_MatchesPkgTbtcGoldenOutput:buildReservationAnchorTransaction(pkg/tbtcpg) andassembleReservationAnchorTransaction(pkg/tbtc) are independentlymaintained copies of the same logic, both unexported in different
packages - Go's visibility rules make a single test calling both
impossible without a production-code change. This test and
pkg/tbtc'sTestAssembleReservationAnchorTransactioninstead pin the identicalgolden input/output values (deposit 100000, fee 1500, output 98500) in
each package, catching either copy drifting from the other. The
underlying duplication itself is unresolved - extracting a shared
exported helper is the real fix, deferred as a larger (L, 1-2 day)
change per decision this session.
Testing
go test ./pkg/tbtc/... ./pkg/tbtcpg/... ./pkg/chain/ethereum/...:519/519 pass.
go build ./...&&go test ./...: full repo, 49 packages, zeroFAIL.gofmt -l/go vet: clean on all 4 changed/new files.Not in this PR
ValidateReservationAnchorProposal/ValidateReservationReanchorProposaltests - deferred, documented in the gap-analysis doc.
buildReservationAnchorTransaction/assembleReservationAnchorTransactionhelper to eliminate theduplication for good - the golden-value tests catch drift but don't fix
the root duplication.