Skip to content

rpc: add includePending option to name_list#586

Open
mstrofnone wants to merge 3 commits into
namecoin:masterfrom
mstrofnone:rfc/name-list-include-pending
Open

rpc: add includePending option to name_list#586
mstrofnone wants to merge 3 commits into
namecoin:masterfrom
mstrofnone:rfc/name-list-include-pending

Conversation

@mstrofnone

Copy link
Copy Markdown

What

Adds an {"includePending": true} option to the name_list RPC that includes the wallet's unconfirmed name operations from the mempool alongside its confirmed names. Default is false, so existing callers see no change.

Why

Today a wallet UI that wants "my names, current state" has to:

  1. Call name_list for the wallet's confirmed names.
  2. Call name_pending (now with mineOnly:true from rpc: add mineOnly to name_pending; fix optional ismine across static archives #585) for the wallet's pending name ops.
  3. Merge them client-side, replacing any confirmed entry whose name has a more recent pending update.

Every Namecoin wallet UI ends up duplicating that merge logic. Even this repo's QT name table model (src/qt/nametablemodel.cpp) does the same dance:

confirmedNames = parent.walletModel->node().executeRpc("name_list", params, walletURI);
...
pendingNames = parent.walletModel->node().executeRpc("name_pending", params, walletURI);

With includePending:true the merge happens server-side, in the place that already has both pieces of data and the right locks.

Behavior

  • Opt-in. Default includePending:false preserves the previous behavior byte-for-byte.
  • When set, name_list also walks pwallet->mapWallet for unconfirmed wallet txs (depth <= 0) and emits an entry per name op.
  • Pending entries are decorated with:
    • "pending": true
    • "op": either "name_firstupdate" or "name_update"
  • A pending entry replaces any confirmed entry for the same name. The wallet's pending op is its more recent intended state, so a UI that respects the pending row matches what the chain will look like once the tx confirms.
  • If multiple pending wallet txs touch the same name (e.g. an RBF chain that hasn't been pruned from the wallet), the one with the highest nOrderPos wins — i.e. the most recently created of the wallet's own pending txs.
  • tx.isAbandoned() pending txs are skipped, mirroring how pwallet->mapWallet consumers usually treat them.

Schema notes

Pending entries have no canonical chain height, so the existing height, expires_in, and expired fields are omitted for them. The result schema now marks those three fields as optional. The output for confirmed entries (with or without includePending) is byte-for-byte unchanged.

The new pending and op fields are also optional and only appear on unconfirmed entries.

Companion changes

This PR is the third leg of three closely-related changes:

This branch is currently stacked on top of #585. The two name-list-side changes (the underlying ismine fix and includePending) are independent, but stacking lets reviewers see the full picture and lets me share the same test infrastructure between the two PRs. If reviewers prefer this PR rebased onto master independently, I'll do it.

Tests

A new functional test, test/functional/name_list_pending.py, covers:

  • the default name_list behavior is unchanged,
  • includePending without any pending wallet tx matches the default,
  • a pending name_update replaces the confirmed entry for that name and carries pending=true, op="name_update",
  • confirmed entries for other names are not touched,
  • the name filter composes with includePending,
  • once the pending tx confirms, the entry loses the pending / op fields and gains real expiration data again.

I ran the full name_*.py battery locally on macOS arm64 with all three commits applied — all 28 tests pass:

TEST                            | STATUS    | DURATION
name_allowexpired.py            | ✓ Passed  |  2 s
name_ant_workflow.py            | ✓ Passed  |  4 s
name_bumpfee.py                 | ✓ Passed  |  2 s
name_byhash.py                  | ✓ Passed  |  1 s
name_encodings.py               | ✓ Passed  | 15 s
name_expiration.py              | ✓ Passed  |  1 s
name_immature_inputs.py         | ✓ Passed  |  8 s
name_ismine.py                  | ✓ Passed  |  3 s
name_list.py                    | ✓ Passed  |  4 s
name_list_pending.py            | ✓ Passed  |  0 s
name_listunspent.py             | ✓ Passed  |  8 s
name_longsalt.py                | ✓ Passed  |  2 s
name_multisig.py                | ✓ Passed  | 16 s
name_multisig.py --bip16-active | ✓ Passed  | 14 s
name_multiupdate.py             | ✓ Passed  |  0 s
name_novalue.py                 | ✓ Passed  |  0 s
name_pending.py                 | ✓ Passed  |  4 s
name_pending_mineonly.py        | ✓ Passed  | 14 s
name_psbt.py                    | ✓ Passed  |  2 s
name_rawtx.py                   | ✓ Passed  |  7 s
name_registration.py            | ✓ Passed  | 18 s
name_reorg.py                   | ✓ Passed  |  1 s
name_scanning.py                | ✓ Passed  |  1 s
name_segwit.py                  | ✓ Passed  |  0 s
name_sendcoins.py               | ✓ Passed  |  0 s
name_txnqueue.py                | ✓ Passed  |  1 s
name_utxo.py                    | ✓ Passed  |  1 s
name_wallet.py                  | ✓ Passed  | 10 s

Build: macOS 15 arm64, clang from Xcode toolchain, CMake build with default flags, ENABLE_WALLET=ON.

RFC notes / questions for review

This is shaped as something close to a draft; happy to iterate on:

  • Tie-break for multiple pending wallet txs on the same name. I went with highest nOrderPos (most-recently-added to the wallet). An alternative is "first one found in the mempool that's not RBF-replaced." nOrderPos keeps the implementation entirely wallet-side without needing to consult the mempool again, and matches what a UI would intuitively want ("show me my latest intent"), but I can switch if reviewers prefer the mempool-anchored interpretation.
  • Pending replaces confirmed. If reviewers prefer to emit both rows (one pending, one confirmed) for clarity, I can make that the default and gate the merge behind a separate flag like mergePending. That said, every wallet UI I've seen wants the merged view, and the schema cost of adding two rows for one name is non-trivial.
  • Mempool-only-not-yet-in-wallet. This PR only includes pending name ops that are in pwallet->mapWallet. It does not include "someone else broadcast a tx that transfers a name to my wallet but the tx isn't in my wallet yet." Adding that would require touching the mempool from name_list, which I'd rather do as a separate change if at all.

mstrofnone added 3 commits May 5, 2026 20:21
When wallet support is enabled, MaybeWalletForRequest is meant to attach
the active wallet to a request so that name_show, name_history, name_scan
and name_pending can decorate their results with an "ismine" field.  The
original implementation gated the wallet lookup on
util::AnyPtr<wallet::WalletContext>(request.context) called from
src/rpc/names.cpp, which lives in the bitcoin_node static archive.

On platforms where typeinfo for wallet::WalletContext ends up duplicated
across the bitcoin_node and bitcoin_wallet archives (notably macOS clang
with the default hidden-typeinfo visibility), the typeid stored in
request.context (set in src/wallet/interfaces.cpp inside bitcoin_wallet)
does not compare equal to the typeid resolved from the node-side TU,
even though the demangled type name string is identical.  As a result,
the AnyPtr check returned nullptr, MaybeWalletForRequest fell through to
"no wallet", and ismine never appeared on those RPCs.

Move the wallet lookup into a new wallet-side helper,
MaybeGetWalletForJSONRPCRequest, so the typeid comparison happens in the
same translation unit that originally stored the WalletContext pointer.
The helper also collapses the "no wallet available" cases (no wallet
context, no wallet loaded, multiple wallets without selection) into a
single null return, which is exactly the contract the existing callers
in MaybeWalletForRequest already wanted.

Verified locally on macOS arm64: name_pending.py, name_list.py,
name_ismine.py, and the rest of the name_*.py functional tests all
pass after this change, and name_show / name_scan again populate
ismine for wallet-owned names.
Add a {"mineOnly": true} option to the name_pending RPC that filters
the result to only the unconfirmed name operations whose output address
is owned by the loaded wallet.  The default is false, preserving the
existing behavior of returning every pending name operation in the
mempool.

Without this option, a wallet UI that polls name_pending to track its
own pending registrations and updates has to walk every pending name
operation in the mempool on every poll and discard most of them client
side.  That is fine on a quiet chain but scales linearly with overall
pending name traffic.  With mineOnly:true the filter happens server
side, before getNameInfo / addOwnershipInfo run, so the wallet pays
roughly O(its own pending ops) instead of O(all pending name ops in
the mempool).

The new option requires a wallet to be loaded.  When ENABLE_WALLET is
defined and no wallet is loaded (or the request is otherwise
ambiguous, e.g. multiple wallets with no /wallet/<name> selection),
the call returns RPC_WALLET_NOT_FOUND with a clear message rather than
silently returning everything.  When ENABLE_WALLET is undefined,
mineOnly:true returns RPC_METHOD_NOT_FOUND.

Also adds test/functional/name_pending_mineonly.py covering:
  * mineOnly:false matches the default,
  * mineOnly:true with a self-owned pending update,
  * a transferred pending update flips ownership across wallets,
  * mineOnly composes with the name filter and the "does not exist"
    case,
  * mineOnly without a wallet errors RPC_WALLET_NOT_FOUND.
Adds a {"includePending": true} option to the name_list RPC that
also includes unconfirmed wallet name operations from the mempool in
the result.  The default remains false, preserving existing behavior.

Motivation: today a wallet UI that wants "my names, current state"
has to call name_list (confirmed only) AND name_pending separately,
then merge them client side.  Every Namecoin wallet UI ends up
duplicating that merge logic, including the QT name table model in
this repo (src/qt/nametablemodel.cpp).

When includePending is set, name_list walks pwallet->mapWallet for
unconfirmed wallet txs in addition to confirmed ones.  For each
unconfirmed name op:
  * the entry is decorated with "pending": true,
  * "op" is set to either "name_firstupdate" or "name_update",
  * a pending entry replaces any confirmed entry for the same name,
    since the wallet's pending op is its more recent intended state,
  * abandoned wallet txs are skipped.

If multiple pending wallet txs touch the same name (e.g. an RBF
chain), the one with the highest nOrderPos wins, which is the most
recently created of the wallet's own pending txs.

The result schema marks height / expires_in / expired as optional
because they are omitted for pending entries (they are meaningless
without a confirmed chain height).  The new "pending" and "op"
fields are also optional and only appear on unconfirmed entries.
The output for confirmed entries when includePending is unset is
byte-for-byte unchanged.

Also adds test/functional/name_list_pending.py covering:
  * the default behavior is unchanged,
  * includePending without any pending tx matches the default,
  * a pending name_update replaces the confirmed entry for that
    name and carries pending=true, op="name_update",
  * confirmed entries for other names are not affected,
  * the name filter composes with includePending,
  * once the pending tx confirms, the entry loses the pending /
    op fields and gains real expiration data again.
@domob1812

Copy link
Copy Markdown

This is based on #585, so #585 should be merged first, right?

@mstrofnone

Copy link
Copy Markdown
Author

yes @domob1812

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