fix: use 330 sat dust limit for P2TR/P2WPKH change outputs#8807
fix: use 330 sat dust limit for P2TR/P2WPKH change outputs#8807Andezion wants to merge 2 commits intoElementsProject:masterfrom
Conversation
Andezion
force-pushed
the
fix-p2tr-dust-limit
branch
from
7397c92 to
55c14f8
Compare
Andezion
force-pushed
the
fix-p2tr-dust-limit
branch
3 times, most recently
from
b5723ec to
67964a0
Compare
|
@Andezion, please remove the first two commits that are not relevant to this PR. |
|
|
||
| sudo rm -f /etc/apt/sources.list.d/azure-cli.list || true | ||
| sudo rm -f /etc/apt/sources.list.d/microsoft-prod.list || true | ||
|
|
There was a problem hiding this comment.
I don't think you need to modify this file.
| TEST_DEBUG: 1 | ||
| run: | | ||
| VALGRIND=1 uv run eatmydata pytest tests/ -n 3 ${PYTEST_OPTS} ${{ matrix.PYTEST_OPTS }} | ||
| VALGRIND=1 uv run eatmydata pytest tests/ -n 1 ${PYTEST_OPTS} ${{ matrix.PYTEST_OPTS }} |
There was a problem hiding this comment.
I don't think you need to modify this file.
| if is_withdraw: | ||
| acct = "external" | ||
| transfer_from = acct | ||
| acct = "external" |
There was a problem hiding this comment.
Can you explain the motivation behind this change?
| assert not l2.daemon.is_in_log('bad reestablish') | ||
|
|
||
| # Remove mock so l2 can sync during teardown | ||
| l2.daemon.rpcproxy.mock_rpc('getblockhash', None) |
There was a problem hiding this comment.
Please explain? This doesn't look related to the PR title.
| # Feerate can't be lower. | ||
| assert actual_feerate > expected_feerate - 2 | ||
| if actual_feerate >= expected_feerate + 2: | ||
| assert actual_feerate >= expected_feerate - 10 |
There was a problem hiding this comment.
This file as well should not be touched. The changes are not related to the title of the PR, and Rusty already addressed that issue (8a60a27)
Lagrang3
left a comment
Lagrang3
left a comment
There was a problem hiding this comment.
First two commits should be removed.
Some changes afterwards seem unnecessary.
Also, the rest of the commits should be squashed into one, there is basically one
change and the rest is just formatting.
…ixes ElementsProject#8395 Fix by checking is_elements: Elements keeps 546 sat, Bitcoin uses 330 sat. Changelog-Fixed: Transactions now correctly create change outputs >= 330 sat for P2TR/P2WPKH instead of absorbing them as fees
Andezion
force-pushed
the
fix-p2tr-dust-limit
branch
from
f64669d to
765fa8a
Compare
|
@Andezion can you update per Eduardo's comments, resolve the conflicts and clear CI by the end of this week so we can add this to 26.06? Release candidate planned for Monday 11 May. If you can't we'll move it to 25.09 |
4 participants
(Fixes #8395)
Problem
When creating transactions, change outputs between 330-546 sat were being treated as dust and absorbed into fees. This caused users to overpay transaction fees.
The issue was that change_amount() function used chainparams->dust_limit (546 sat) for all output types, but P2TR and P2WPKH outputs have a lower dust limit of 330 sat according to Bitcoin Core standards.
Solution
Changed change_amount() in bitcoin/tx.c to use 330 sat dust limit for change outputs. This is correct because:
Now change outputs between 330-546 sat are properly created instead of being absorbed into fees.
Testing
Important
26.04 FREEZE March 11th: Non-bugfix PRs not ready by this date will wait for 26.06.
RC1 is scheduled on March 23rd
The final release is scheduled for April 15th.
Checklist
Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:
tools/lightning-downgrade