Skip to content

Fix: allow zero-fee transactions in LowestFee metric#35

Merged
evanlinjin merged 3 commits intobitcoindevkit:masterfrom
evanlinjin:fix/lowest-fee-metric-with-zero-fee
Sep 15, 2025
Merged

Fix: allow zero-fee transactions in LowestFee metric#35
evanlinjin merged 3 commits intobitcoindevkit:masterfrom
evanlinjin:fix/lowest-fee-metric-with-zero-fee

Conversation

@evanlinjin
Copy link
Copy Markdown
Member

@evanlinjin evanlinjin commented Sep 15, 2025

Change assertion from fee > 0 to fee >= 0 to handle valid zero-fee transactions. Add test case to verify the metric works correctly when target fee rate is zero.

Also make clippy happy and upgrade test dependencies.

@evanlinjin evanlinjin requested a review from LLFourn September 15, 2025 03:41
evanlinjin and others added 3 commits September 15, 2025 04:57
Change assertion from fee > 0 to fee >= 0 to handle valid zero-fee
transactions. Add test case to verify the metric works correctly
when target fee rate is zero.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@evanlinjin evanlinjin force-pushed the fix/lowest-fee-metric-with-zero-fee branch 2 times, most recently from eddfb94 to 67a8698 Compare September 15, 2025 05:04
@evanlinjin evanlinjin merged commit 23b1cb5 into bitcoindevkit:master Sep 15, 2025
5 checks passed
Comment thread tests/lowest_fee.rs
}

#[test]
fn zero_fee_tx() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure if it matters, but AFAICT this test produces a solution with a non-zero fee (130 sats).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If it's a solution with change then it's a problem. If it's without change, it's expected as we have a long-term feerate so that greatly disincentives the solution with change.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

But maybe we should check that a solution with change has 0 fee.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah OK, the solution doesn't have change so that's fine. I wasn't sure if the intention of the test was to produce a transaction with zero fee.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants