taproot-assets
taproot-assets copied to clipboard
Limit number of HTLCs in custom channel
Depends on https://github.com/lightninglabs/taproot-assets/pull/1130.
Addresses comment in https://github.com/lightningnetwork/lnd/pull/9072#discussion_r1765319065.
Calculated max values from TestMaxCommitSigMsgSize:
aux_leaf_signer_test.go:111: Last valid commit sig msg size with: numAssetIDs=0, numHTLCs=966
aux_leaf_signer_test.go:103: Last valid commit sig msg size with: numAssetIDs=1, numHTLCs=373
aux_leaf_signer_test.go:103: Last valid commit sig msg size with: numAssetIDs=2, numHTLCs=231
aux_leaf_signer_test.go:103: Last valid commit sig msg size with: numAssetIDs=3, numHTLCs=166
aux_leaf_signer_test.go:103: Last valid commit sig msg size with: numAssetIDs=4, numHTLCs=130
aux_leaf_signer_test.go:103: Last valid commit sig msg size with: numAssetIDs=5, numHTLCs=107
aux_leaf_signer_test.go:103: Last valid commit sig msg size with: numAssetIDs=6, numHTLCs=91
aux_leaf_signer_test.go:103: Last valid commit sig msg size with: numAssetIDs=7, numHTLCs=79
aux_leaf_signer_test.go:103: Last valid commit sig msg size with: numAssetIDs=8, numHTLCs=70
aux_leaf_signer_test.go:103: Last valid commit sig msg size with: numAssetIDs=9, numHTLCs=63
aux_leaf_signer_test.go:103: Last valid commit sig msg size with: numAssetIDs=10, numHTLCs=57
Pull Request Test Coverage Report for Build 11935690852
Details
- 0 of 84 (0.0%) changed or added relevant lines in 4 files are covered.
- 18 unchanged lines in 7 files lost coverage.
- Overall coverage decreased (-0.04%) to 41.263%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| tapcfg/server.go | 0 | 1 | 0.0% |
| server.go | 0 | 2 | 0.0% |
| psbt_channel_funder.go | 0 | 21 | 0.0% |
| tapchannel/aux_funding_controller.go | 0 | 60 | 0.0% |
| <!-- | Total: | 0 | 84 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| tapchannel/aux_funding_controller.go | 1 | 0.0% |
| asset/mock.go | 1 | 92.51% |
| commitment/tap.go | 2 | 84.43% |
| asset/asset.go | 2 | 81.13% |
| tapchannel/aux_leaf_signer.go | 3 | 36.33% |
| tapdb/universe.go | 4 | 80.91% |
| universe/interface.go | 5 | 50.65% |
| <!-- | Total: | 18 |
| Totals | |
|---|---|
| Change from base Build 11920687788: | -0.04% |
| Covered Lines: | 25469 |
| Relevant Lines: | 61723 |
💛 - Coveralls
Ok, discussed offline a bit, and I understand now the need to limit on the asset ID basis. Rather than asset ID, it's more accurate to say that we need to limit the total number of UTXOs that can exist within a funding output. Each time we create an HTLC, we may need to reference one or more asset UTXOs from the funding output. Each of those means an input referenced, which itself needs a signature. As a result, a single HTLC may actually need several HTLCs to be transmitted over the wire.
Without moving to a more efficient encoding (not much room can be gained), or to a new way of transmitting the signatures, then we need to limit the total amount of asset UTXOs that can reside within a funding output, or we run into message size limits on the protocol level.
One alternative path is to add the equivalent of SIGHASH_NOINPUT on the protocol layer. If we add this, then when we send HTLCs, we can actually send the second-level signatrues alongside them, as we just need the pkScript to be stable, which is the case for the next commitment. I think this breaks down though, as for the commitment after that, a different per-commitment-secret is used (new revocation secret), so that original signature can no longer be used.
Rebased to make sure CI runs against new litd branch. Also taking out of draft state since the discussions showed we want this safety feature as a short-term stop-gap solution.
@roasbeef: review reminder @guggero, remember to re-request review from reviewers when ready
Looks like we forgot to add the commitment type to the channel acceptor. Didn't notice this litd itest failure before, since it ran against the wrong branch.
So we'll need to wait for https://github.com/lightningnetwork/lnd/pull/9288 and then bump the lnd version.