-
Notifications
You must be signed in to change notification settings - Fork 1
Support 0.1 sat/vbyte minimum fee rate from Bitcoin Core 29.1+ #10
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
Conversation
Updates the bucket system to handle the new 0.1 sat/vbyte minimum fee rate introduced in Bitcoin Core 29.1/30.0 (PR #33106). The logarithmic bucketing now supports negative bucket indices down to -230 (corresponding to 0.1 sat/vB). Changes: - Add BUCKET_MIN constant (-230) for 0.1 sat/vbyte threshold - Increase BUCKET_ARRAY_SIZE from 1001 to 1231 to accommodate negative indices - Update filter logic to accept transactions >= 0.1 sat/vbyte - Fix array out of bounds issue when mapping bucket indices to array positions - Add test coverage for very low fee rate transactions (0.1, 0.2 sat/vB)
| val bucket02 = (ln(0.2) * 100).roundToInt() // ~-161 | ||
| val bucket1 = 0 // ln(1) * 100 = 0 | ||
|
|
||
| println("Buckets: $buckets") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| snapshot.bucketedWeights.forEach { (bucket, weight) -> | ||
| // Remove buckets that are less than 0 (i.e. fee rates that are less than 1 satoshi/vByte) | ||
| if (bucket >= 0) { | ||
| // Remove buckets that are less than BUCKET_MIN (i.e. fee rates that are less than 0.1 satoshi/vByte) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit but maybe change 0.1 to ~0.1 (since with the rounding, we'll actually still accept 0.0998).
Side note - is accepting 0.0998 alright? I think so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Speaking of, maybe we should add a test file MempoolSnapshotF64ArrayTest to confirm that a 0.05 fee rate gets ignored but doesn't break anything. Claude suggests this?
@OptIn(InternalAugurApi::class)
class MempoolSnapshotF64ArrayTest {
@Test
fun `fromMempoolSnapshot drops buckets below minimum`() {
val lowBucket = BucketCreator.BUCKET_MIN - 1
val validBucket = BucketCreator.BUCKET_MIN
val snapshot = MempoolSnapshot(
blockHeight = 100,
timestamp = Instant.now(),
bucketedWeights = mapOf(
lowBucket to 400L,
validBucket to 600L,
),
)
val result = MempoolSnapshotF64Array.fromMempoolSnapshot(snapshot)
assertEquals(BucketCreator.BUCKET_ARRAY_SIZE, result.buckets.length)
val validIndex = BucketCreator.BUCKET_MAX - validBucket
assertEquals(600.0, result.buckets[validIndex])
var totalWeight = 0.0
for (i in 0 until result.buckets.length) {
totalWeight += result.buckets[i]
}
assertEquals(600.0, totalWeight)
assertTrue(validIndex == result.buckets.length - 1)
}
@Test
fun `fromMempoolSnapshot ignores very low fee rates like 0_05 sat per vB`() {
// 0.05 sat/vB is below Bitcoin Core's minimum relay fee (0.1 sat/vB in 29.1+)
// This should be gracefully ignored, not cause an error
val veryLowFeeRate = 0.05
val veryLowBucket = (ln(veryLowFeeRate) * 100).roundToInt() // -300
// Verify this bucket is indeed below BUCKET_MIN
assertTrue(veryLowBucket < BucketCreator.BUCKET_MIN)
val validBucket = 0 // 1 sat/vB
val snapshot = MempoolSnapshot(
blockHeight = 100,
timestamp = Instant.now(),
bucketedWeights = mapOf(
veryLowBucket to 1000L,
validBucket to 500L,
),
)
val result = MempoolSnapshotF64Array.fromMempoolSnapshot(snapshot)
// Should not throw, and should only include the valid bucket's weight
var totalWeight = 0.0
for (i in 0 until result.buckets.length) {
totalWeight += result.buckets[i]
}
assertEquals(500.0, totalWeight)
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Side note - is accepting 0.0998 alright? I think so?
yeah that's fine, since that'll eventually emit a little over 0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that'll eventually emit a little over 0.1
Explain how we will end up over 0.1?
We should make sure we never emit a fee rate of e.g. 0.0998 (less than 0.1) because that would prevent the transaction from getting forwarded with the default bitcoin core settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
He explained that in his new comment I think:
// Remove buckets below BUCKET_MIN (~0.1 sat/vB; round() admits ~0.0998 here,
// which converts back to ~0.10026 sat/vB so we never emit a sub-0.1 estimate)
Can we add a test like this to FeeEstimatesCalculatorTest to demonstrate your comment Kyle?
@Test
fun `test near-minimum fee bucket never emits sub 0_1 sat per vB`() {
val nearMinimumFeeRate = 0.0998
val bucketIndex = (ln(nearMinimumFeeRate) * 100).roundToInt()
assertEquals(BUCKET_MIN, bucketIndex)
val snapshot =
MempoolSnapshot(
blockHeight = 800_000,
timestamp = Instant.EPOCH,
bucketedWeights = mapOf(bucketIndex to 4_000_000L),
)
val mempoolBuckets = MempoolSnapshotF64Array.fromMempoolSnapshot(snapshot).buckets
val zeroInflows = F64Array(BucketCreator.BUCKET_ARRAY_SIZE) { 0.0 }
val estimates = calculator.getFeeEstimates(
mempoolBuckets,
zeroInflows,
zeroInflows.copy(),
)
val expectedFeeRate = exp(bucketIndex.toDouble() / 100.0)
estimates.forEachIndexed { blockIdx, row ->
row.forEachIndexed { probIdx, fee ->
val actual = requireNotNull(fee)
assertTrue(actual >= 0.1, "estimate[$blockIdx][$probIdx] should be >= 0.1 sat/vB")
assertEquals(
expectedFeeRate,
actual,
1e-12,
"estimate[$blockIdx][$probIdx] should match the rounded bucket fee rate",
)
}
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test added
…returns BUCKET_MIN, and tests expect the 0.1 sat/vB bucket (lib/src/main/kotlin/xyz/block/augur/internal/ FeeEstimatesCalculator.kt, lib/src/test/kotlin/xyz/block/augur/internal/FeeEstimatesCalculatorTest.kt). - Clarified low-fee handling: comment now notes round() can admit ~0.0998 but it converts back to ~0.10026, so we never emit sub-0.1 and stay above relay floor (lib/src/main/ kotlin/xyz/block/augur/internal/MempoolSnapshotF64Array.kt). - Addressed PR feedback: removed debug println in low-fee bucket test, added MempoolSnapshotF64ArrayTest to assert we drop buckets below BUCKET_MIN and ignore a 0.05 sat/ vB entry, and kept the existing very-low-fee coverage (lib/src/test/kotlin/xyz/block/augur/internal/BucketCreatorTest.kt, lib/src/test/kotlin/xyz/block/augur/internal/ MempoolSnapshotF64ArrayTest.kt).
laurenshareshian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we're ready to merge this I think we'll also want to bump VERSION_NAME=0.2.1.
bumped to 0.2.2 |
Updates the bucket system to handle the new 0.1 sat/vbyte minimum fee rate introduced in Bitcoin Core 29.1/30.0 (PR #33106). The logarithmic bucketing now supports negative bucket indices down to -230 (corresponding to 0.1 sat/vB).
Changes: