Emit getprioritisedtransactions modified_fee in satoshis by metaphorics · Pull Request #605 · gosuda/bitcoin-rs

metaphorics · 2026-09-04T20:00:32Z

Stacked on #604. Core v31 getprioritisedtransactions (src/rpc/mining.cpp) documents modified_fee as a NUM in satoshis, and mining RPCs follow GBT in using satoshi amounts rather than BTC.

This node already emitted fee_delta as an integer satoshi overlay and omitted modified_fee when in_mempool is false. Pooled modified_fee (actual fee plus delta) was projected through signed_sat_to_btc, so clients saw 0.000015 instead of 1500. Projection now emits a JSON integer in satoshis.

Contract: API-25. Proven by getprioritisedtransactions_projects_the_overlay. getmempoolentry.modified stays BTC; that method is not a mining RPC.

Does not complete #151. Remaining mining work includes the live Core differential suite (#78), numeric p95 budgets (#158), -acceptnonstdtxn as a real option, and prune BLOCK_HAVE_DATA vs scripts-valid.

Core mining RPCs use satoshi amounts. fee_delta was already an integer overlay; modified_fee was incorrectly projected as BTC. Emit actual fee plus delta as a JSON number in satoshis, omitting the field when the txid is not in the mempool. Co-authored-by: metaphorics <metaphorics@users.noreply.github.com>

coderabbitai · 2026-09-04T20:00:39Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: d8f49282-8c46-428f-9bae-6a870a9363fa

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

cursor · 2026-09-04T20:00:43Z

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_c080bc16-1245-45a6-a7b6-0eb29d175254)

qodo-code-review · 2026-09-04T20:01:29Z

PR Summary by Qodo

Emit prioritised transaction modified fees in satoshis

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 10-20 Minutes

AI Description

• Emit pooled modified_fee as integer satoshis for Core-compatible mining RPC responses.
• Keep modified_fee absent for prioritised transactions outside the mempool.
• Document API-25 and update regression coverage and the RPC reference.

Diagram

sequenceDiagram
    actor Client
    participant RPC as Mining RPC
    participant Pool as Mempool
    Client->>RPC: getprioritisedtransactions
    RPC->>Pool: Read fee overlay
    Pool-->>RPC: Fee and delta
    RPC->>RPC: Convert to i64 sats
    RPC-->>Client: modified_fee 1500

High-Level Assessment

The current approach is appropriate: conversion occurs at the RPC boundary, preserves the mempool model’s signed i128 arithmetic, saturates to Core’s signed 64-bit CAmount representation, and retains field omission for non-pooled transactions. Continuing to serialize through BTC would violate the mining RPC contract, while exposing the internal i128 directly would not match Core’s wire semantics.

Files changed (5) +35 / -14

Bug fix (1) +18 / -10

mining.rsSerialize modified fees as integer satoshis +18/-10

Serialize modified fees as integer satoshis

• Replaces BTC floating-point projection with a saturating signed 64-bit satoshi value for pooled 'modified_fee' fields. Updates the overlay regression test to assert the integer value and links implementation and coverage to API-25.

crates/rpc/src/handlers/mining.rs

Documentation (4) +17 / -4

registry.rsClarify getprioritisedtransactions fee units +1/-1

Clarify getprioritisedtransactions fee units
• Extends the RPC registry description to state that 'modified_fee' uses satoshis rather than BTC, matching Bitcoin Core mining RPC conventions.
crates/rpc/src/registry.rs

README.mdRegister external API contract API-25 +1/-1

Register external API contract API-25

• Expands the external API contract range through API-25 and records the 'getprioritisedtransactions' satoshi-unit guarantee in the contract index.

docs/contracts/README.md

external-api.mdDefine the modified_fee satoshi contract +14/-1

Define the modified_fee satoshi contract

• Adds API-25, specifying that pooled 'modified_fee' is actual fee plus delta expressed as a JSON satoshi number. Associates the contract with the mining handler regression test.

docs/contracts/external-api.md

rpc-reference.mdDocument modified_fee as satoshis +1/-1

Document modified_fee as satoshis

• Updates the generated RPC reference entry for 'getprioritisedtransactions' to distinguish its satoshi-denominated 'modified_fee' from BTC-denominated fee fields elsewhere.

docs/rpc-reference.md

qodo-code-review · 2026-09-04T20:02:14Z

Code Review by Qodo

📘 Rule violations (2)

Issue findings (2)

Findings have been published as inline review comments

 ⓘ Findings that repeat in the code will be published as multiple comments

qodo-code-review · 2026-09-04T20:02:15Z

    "generateblock", SurfaceKind::Rpc, Status::Implemented, "", CORE_VERSION, "Assembles and solves one block paying an address or descriptor from the listed mempool txids or raw txs in that order; third param is Core's submit flag. Unknown 64-hex txids are -5; raw-tx decode failures are -22.", "0.4.0", Some(mining::generateblock);
    "getnetworkhashps", SurfaceKind::Rpc, Status::Implemented, "", CORE_VERSION, "Estimated hashes/s over a caller-chosen lookback ending at a caller-chosen height; default lookback 120, height the applied tip.", "0.4.0", Some(mining::getnetworkhashps);
-    "getprioritisedtransactions", SurfaceKind::Rpc, Status::Implemented, "", CORE_VERSION, "Projects the mempool's signed fee-delta overlay, including txids not currently pooled.", "0.4.0", Some(mining::getprioritisedtransactions);
+    "getprioritisedtransactions", SurfaceKind::Rpc, Status::Implemented, "", CORE_VERSION, "Projects the mempool's signed fee-delta overlay, including txids not currently pooled. modified_fee is satoshis like Core mining RPCs, not BTC.", "0.4.0", Some(mining::getprioritisedtransactions);


1. modified_fee rule duplicated 📘 Rule violation ⚙ Maintainability

The registry metadata independently restates the modified_fee unit rule already defined by API-25, without referencing that authoritative contract. This creates parallel documentation that can drift when the API contract changes.

Agent Prompt

## Issue description The `getprioritisedtransactions` registry note duplicates the `modified_fee` unit rule defined in the authoritative `API-25` contract. ## Issue Context Keep the registry description concise and refer to `docs/contracts/external-api.md#API-25` rather than independently restating the satoshi-versus-BTC rule. Regenerate `docs/rpc-reference.md` afterward because it is generated from the registry. ## Fix Focus Areas - crates/rpc/src/registry.rs[146-146] - docs/rpc-reference.md[80-80]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

qodo-code-review · 2026-09-04T20:02:15Z

+            let sats = i64::try_from(modified_fee).unwrap_or_else(|_| {
+                if modified_fee.is_negative() {
+                    i64::MIN
+                } else {


2. Signed saturation logic duplicated 📘 Rule violation ⌂ Architecture

The new i128-to-i64 saturation formula duplicates the conversion already embedded in signed_sat_to_btc instead of using one canonical conversion helper. Future changes to RPC numeric-boundary behavior could therefore diverge between the two paths.

Agent Prompt

## Issue description The new `modified_fee` projection repeats signed `i128`-to-`i64` saturation logic already present in `signed_sat_to_btc`. ## Issue Context Introduce a canonical signed saturation helper in the RPC conversion module, use it from both `signed_sat_to_btc` and `getprioritisedtransactions`, and retain the existing boundary behavior. ## Fix Focus Areas - crates/rpc/src/compat/convert.rs[68-77] - crates/rpc/src/handlers/mining.rs[290-296]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

qodo-code-review · 2026-09-04T20:03:12Z

Qodo Fixer

🍒 Ready to be cherry-picked — ✅ Merged (0) · ☑ Fixed (2)

🔗 Fix PR: #607

This fix PR was closed automatically. Its branch is preserved so you can cherry pick the changes into the original PR.

Prompt for coding agent

This is an automated fix prepared on a separate branch (#607). It is NOT applied to this PR.
To use it: review Fix PR #607 (https://github.com/gosuda/bitcoin-rs/pull/607), evaluate each change critically against your local context, and cherry-pick the changes that are correct into this branch. Do not accept them blindly.

Process — 2 fixed

☑ Fixed: Signed saturation logic duplicated
☑ Fixed: modified_fee rule duplicated

qodo-code-review Bot added the 🟠 Medium blast radius label Sep 4, 2026

qodo-code-review Bot reviewed Sep 4, 2026

View reviewed changes

qodo-code-review Bot mentioned this pull request Sep 4, 2026

Fix: [for cherry-picking] Added a shared signed satoshi-to-i64 saturati #607

Open

metaphorics mentioned this pull request Sep 4, 2026

Match Core generate invalid-output error text #608

Draft

Navigation Menu

Uh oh!

Emit getprioritisedtransactions modified_fee in satoshis - #605

Emit getprioritisedtransactions modified_fee in satoshis#605
metaphorics wants to merge 1 commit into
cursor/mining-generateblock-tx-errors-1522from
cursor/mining-prioritised-sats-1522

metaphorics commented Sep 4, 2026

Uh oh!

coderabbitai Bot commented Sep 4, 2026

Review skipped

Uh oh!

cursor Bot commented Sep 4, 2026

Uh oh!

qodo-code-review Bot commented Sep 4, 2026

Uh oh!

qodo-code-review Bot commented Sep 4, 2026

Uh oh!

qodo-code-review Bot Sep 4, 2026

Uh oh!

qodo-code-review Bot Sep 4, 2026

Uh oh!

qodo-code-review Bot commented Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

metaphorics commented Sep 4, 2026

Uh oh!

coderabbitai Bot commented Sep 4, 2026

Review skipped

Uh oh!

cursor Bot commented Sep 4, 2026

Bugbot couldn't run - usage limit reached

Uh oh!

qodo-code-review Bot commented Sep 4, 2026

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Sep 4, 2026

Code Review by Qodo

Issue findings (2)

Uh oh!

qodo-code-review Bot Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-code-review Bot Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-code-review Bot commented Sep 4, 2026

Qodo Fixer

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants