Skip to content

Conversation

guggero
Copy link
Contributor

@guggero guggero commented Aug 5, 2025

Fixes three issues with the integration tests that only surface sometimes.

guggero added 3 commits August 5, 2025 16:07
Fixes an itest case where shutting down the client before a server
connection to the auth mailbox server could be established lead to the
cancellation being treated as a critical error, causing the test to
fail.
When CPU cycles are constrained (e.g. in CI), it can sometimes happen
that we:
 - Import a proof into the multi archiver
 - The multi archiver imports it into each backend
 - The first backend notifies its subscribers after importing
 - The custodian reacts to the notification and checks HasProof on
   the multi archiver
 - Because the other goroutine hasn't finished importing into all
   backends, HasProof returns false
 - The custodian only finishes the transfer once it receives the
   notification from the second backend

All the above leads to the situation where the status "proof received"
is fired twice instead of just once, which causes the itests to fail
sometimes.
@guggero guggero requested review from ffranr and GeorgeTsagk August 5, 2025 14:30
@coveralls
Copy link

coveralls commented Aug 5, 2025

Pull Request Test Coverage Report for Build 16753041762

Details

  • 20 of 32 (62.5%) changed or added relevant lines in 2 files are covered.
  • 55 unchanged lines in 13 files lost coverage.
  • Overall coverage increased (+0.02%) to 56.492%

Changes Missing Coverage Covered Lines Changed/Added Lines %
proof/archive.go 20 23 86.96%
authmailbox/receive_subscription.go 0 9 0.0%
Files with Coverage Reduction New Missed Lines %
fn/context_guard.go 1 88.71%
proof/archive.go 1 60.68%
fn/iter.go 2 62.07%
tapdb/sqlc/transfers.sql.go 2 83.22%
tapgarden/custodian.go 2 76.17%
itest/multisig.go 3 97.91%
tapdb/assets_common.go 3 78.19%
mssmt/compacted_tree.go 4 81.01%
rpcserver.go 4 61.46%
tapgarden/caretaker.go 4 77.3%
Totals Coverage Status
Change from base Build 16746535709: 0.02%
Covered Lines: 59241
Relevant Lines: 104866

💛 - Coveralls

Copy link
Member

@GeorgeTsagk GeorgeTsagk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK (at least didn't run into any new flakes)

@github-project-automation github-project-automation bot moved this from 🆕 New to 👀 In review in Taproot-Assets Project Board Aug 5, 2025
@GeorgeTsagk GeorgeTsagk added this pull request to the merge queue Aug 5, 2025
Merged via the queue into main with commit 57da71b Aug 5, 2025
35 of 38 checks passed
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Taproot-Assets Project Board Aug 5, 2025
@guggero guggero deleted the itest-fixes branch August 5, 2025 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants