Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup Asset Minting Code in Preparation for Universe Commitments #1324

Merged
merged 5 commits into from
Jan 27, 2025

Conversation

ffranr
Copy link
Contributor

@ffranr ffranr commented Jan 24, 2025

Changes are non-functional but should improve code clarity and robustness.

  • enforce output count validation in extractAnchorOutputIndex
  • add nil check for group anchor validation
  • improve documentation for PendingBatch and prepAssetSeedling
  • refine rpc log messages for clarity

ffranr and others added 5 commits January 23, 2025 18:19
Remove inaccurate statement in `prepAssetSeedling` doc comment.

Improve clarity of comment at `prepAssetSeedling` call site.
Update doc comment for PendingBatch to improve clarity. Test to verify
that nil is returned when no pending batch exists.
Introduce a nil pointer check for group anchors to safeguard
against potential nil pointer dereference during validation.

While all current call sites already perform this check, this
change is preemptive and should not have any functional impact
at this time.
Update extractAnchorOutputIndex to raise an error when the number of
unsigned transaction outputs is not exactly two. The function logic
assumes two outputs, so this change preemptively guards against
potential developer errors if additional outputs are ever introduced.
@coveralls
Copy link

Pull Request Test Coverage Report for Build 12957338960

Details

  • 16 of 31 (51.61%) changed or added relevant lines in 4 files are covered.
  • 11 unchanged lines in 5 files lost coverage.
  • Overall coverage increased (+0.02%) to 40.73%

Changes Missing Coverage Covered Lines Changed/Added Lines %
rpcserver.go 0 1 0.0%
tapgarden/batch.go 1 3 33.33%
tapgarden/caretaker.go 5 11 45.45%
tapgarden/planter.go 10 16 62.5%
Files with Coverage Reduction New Missed Lines %
tapgarden/caretaker.go 1 68.11%
tappsbt/create.go 2 53.22%
tapchannel/aux_leaf_signer.go 2 43.08%
asset/asset.go 2 77.06%
commitment/tap.go 4 83.86%
Totals Coverage Status
Change from base Build 12934944306: 0.02%
Covered Lines: 26730
Relevant Lines: 65627

💛 - Coveralls

@ffranr ffranr requested review from guggero and removed request for GeorgeTsagk January 27, 2025 09:14
Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Nice, LGTM 🎉

Copy link
Contributor

@gijswijs gijswijs left a comment

Choose a reason for hiding this comment

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

tACK, LGTM! 🎉

@ffranr ffranr added this pull request to the merge queue Jan 27, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 27, 2025
@ffranr ffranr added this pull request to the merge queue Jan 27, 2025
Merged via the queue into main with commit 6c73a87 Jan 27, 2025
18 checks passed
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