Skip to content

Conversation

Roasbeef
Copy link
Member

No description provided.

@Roasbeef Roasbeef added documentation Improvements or additions to documentation burning no-changelog labels Sep 10, 2025
@coveralls
Copy link

Pull Request Test Coverage Report for Build 17600559753

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 49 unchanged lines in 16 files lost coverage.
  • Overall coverage increased (+0.01%) to 56.997%

Files with Coverage Reduction New Missed Lines %
commitment/tap.go 1 85.65%
address/mock.go 2 96.2%
asset/asset.go 2 80.3%
asset/group_key.go 2 72.15%
tapdb/addrs.go 2 78.23%
tapdb/sqlc/transfers.sql.go 2 83.33%
tapdb/sqlc/universe.sql.go 2 75.78%
tapdb/universe.go 2 81.9%
tapgarden/custodian.go 2 77.02%
universe_rpc_diff.go 2 76.0%
Totals Coverage Status
Change from base Build 17562325369: 0.01%
Covered Lines: 63204
Relevant Lines: 110891

💛 - Coveralls

@Roasbeef Roasbeef requested a review from ffranr September 17, 2025 00:06
@lightninglabs-deploy
Copy link

@ffranr: review reminder

```

When a burn request arrives, the system first performs comprehensive input
validation. The implementation supports both binary and hex string asset IDs,
Copy link
Contributor

Choose a reason for hiding this comment

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

Just adding a note that if this gets merged after #1812, then we should change asset IDs by asset IDs or group keys or similar.

Comment on lines +112 to +117
Following initial validation, the system attempts to resolve the asset's group
membership. This resolution step, while currently limited in its application due
to the single-asset restriction, lays the groundwork for future group-based burn
operations. The system queries the asset store to identify whether the target
asset belongs to a group, information that becomes crucial for proper burn key
derivation and commitment management.
Copy link
Contributor

Choose a reason for hiding this comment

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

Whole paragraph becomes unnecessary if the PR goes after #1812

preventing scenarios where insufficient funds or incorrect asset types might
lead to failed transactions.

Coin selection follows, utilizing the PreferMaxAmount strategy to minimize the
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Coin selection follows, utilizing the PreferMaxAmount strategy to minimize the
Coin selection follows, utilizing the `PreferMaxAmount` strategy to minimize the

Verbatim

Comment on lines +170 to +175
firstPrevID := asset.PrevID{
OutPoint: firstInput.AnchorPoint,
ID: firstInput.Asset.ID(),
ScriptKey: asset.ToSerialized(firstInput.Asset.ScriptKey.PubKey),
}
burnKey := asset.NewScriptKey(asset.DeriveBurnKey(firstPrevID))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
firstPrevID := asset.PrevID{
OutPoint: firstInput.AnchorPoint,
ID: firstInput.Asset.ID(),
ScriptKey: asset.ToSerialized(firstInput.Asset.ScriptKey.PubKey),
}
burnKey := asset.NewScriptKey(asset.DeriveBurnKey(firstPrevID))
firstPrevID := pkt.Inputs[0].PrevID
burnKey := asset.NewScriptKey(asset.DeriveBurnKey(firstPrevID))

I think we can omit the rest that doesn't seem relevant to the paragraph.

transfers that might create tombstones.

Input commitment allocation represents one of the most complex aspects of the
burn process. The createFundedPacketWithInputs function maps selected
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
burn process. The createFundedPacketWithInputs function maps selected
burn process. The `createFundedPacketWithInputs` function maps selected

Comment on lines +195 to +201
The system enforces several critical constraints during execution. The full burn
prevention mechanism blocks attempts to burn all assets in an anchor output,
preventing the creation of empty outputs that would waste Bitcoin UTXO space.
The single packet restriction currently limits burns to one virtual packet,
effectively preventing group-based burns. These constraints, while limiting in
some scenarios, ensure system stability and prevent edge cases that could
compromise the burn mechanism's integrity.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The system enforces several critical constraints during execution. The full burn
prevention mechanism blocks attempts to burn all assets in an anchor output,
preventing the creation of empty outputs that would waste Bitcoin UTXO space.
The single packet restriction currently limits burns to one virtual packet,
effectively preventing group-based burns. These constraints, while limiting in
some scenarios, ensure system stability and prevent edge cases that could
compromise the burn mechanism's integrity.

Unnecessary paragraph after #1812

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
burning documentation Improvements or additions to documentation no-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants