Skip to content

[Fix / Upgrade] PoS Staking tx inclusion, watchonly upgrades, new stealthkey rpc#1053

Open
blondfrogs wants to merge 7 commits into
Veil-Project:masterfrom
blondfrogs:watchonlyaddressremovefix
Open

[Fix / Upgrade] PoS Staking tx inclusion, watchonly upgrades, new stealthkey rpc#1053
blondfrogs wants to merge 7 commits into
Veil-Project:masterfrom
blondfrogs:watchonlyaddressremovefix

Conversation

@blondfrogs

@blondfrogs blondfrogs commented Jan 9, 2026

Copy link
Copy Markdown
Collaborator

This PR contains several fixes and improvements for watch-only address handling, stealth key management, and a critical PoS block creation bug.

Bug Fixes

Fix PoS blocks dropping first mempool transaction (7e90fcf)

  • PoS blocks were silently dropping the first transaction from the mempool
  • The coinstake transaction was overwriting position 1 in the block's transaction list, where addPackageTxs() had already placed a mempool transaction
  • Added a dummy coinstake placeholder to reserve position 1 before mempool transactions are added
  • This ensures all pending transactions are properly included in PoS blocks

Fix watchonly status showing 0 transactions (1c4c634)

  • getwatchonlystatus was reporting 0 transactions even when transactions existed
  • Legacy bug caused first transaction to be stored at index 0 with count also stored as 0
  • Added check to detect transactions at index 0 when count is 0

Fix watchonly status and indexing (af9de02)

  • Fixed sync status check to compare against heightWhenImported instead of scanFromHeight
  • Added progress information (scanned height, scan from height, imported height, percent complete)
  • Fixed getwatchonlytxes indexing to use proper 0-based user-facing API with 1-based database indexing
  • Added input validation for starting_index parameter

New Features

Import stealth keys RPC and dump improvements (49affbf)

  • Added importstealthkeys RPC to import stealth addresses with full private keys (scan + spend secrets)
  • Added stealth key support to dumpwallet RPC - now exports stealth address private keys
  • Fixed metadata saving - ephemeral pubkeys are now always saved (needed to regenerate keys later)
  • Fixed several instances of RegenerateKey to use GetKey for proper key retrieval

Remove watchonly address improvements (c43a625)

  • Enhanced removewatchonlyaddress to accept stealth address string directly (in addition to scan_secret + spend_public)
  • Now properly removes all associated transaction data from the database
  • Returns count of transactions removed
  • Added database method EraseWatchOnlyAddress for complete cleanup

@ohcee

ohcee commented Jan 10, 2026

Copy link
Copy Markdown
Collaborator

utACK

@blondfrogs blondfrogs force-pushed the watchonlyaddressremovefix branch from 85c58cb to 49affbf Compare January 19, 2026 23:37
@blondfrogs blondfrogs changed the title [Bug Fix] Ability to remove watchonly addresses fully [Fix / Upgrade] PoS Staking tx inclusion, watchonly upgrades, new stealthkey rpc Jan 26, 2026

@seanPhill seanPhill left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I appreciate this bug fix (c43a625 "remove watchonly address fixes") and will be testing it on testnet pronto.
utACK

@seanPhill seanPhill left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK af9de02 "watchonly status and indexing"

@seanPhill seanPhill left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK ( 49affbf "importstealth rpc, stealthkey support in dump rpc, metadata saved")
Love your code quality and documentation!

@seanPhill seanPhill left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK ( 7e90fcf "Fix PoS blocks dropping first mempool transaction" )
This is a great catch! I had not realised why I often had to push through transactions on testnet with a proof of work block. On mainnet it wasn't a huge problem, as there were always proof of work blocks. Testing this on testnet shortly.

In fact it wasn't extremely obvious as just looking at my spreadsheet of the superblock transactions, they've been confirmed in staked blocks slightly more than 50% of the time, and I haven't analysed this on other transactions such as budgets or my own many spends, statistically, so I hardly ever noticed it on mainnet, just when spending testnet funds, or noticed the wait occasionally when broadcasting a budget spend, and didn't attribute it to anything like this.

@seanPhill seanPhill left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK ( 1c4c634 "Fix watchonly status showing 0 transactions" )
💯

@Veil-Project

Copy link
Copy Markdown
Owner

Very much appreciated. Will merge this as soon as testnet testing has been done.

@ohcee

ohcee commented Jan 27, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the awesome work @blondfrogs.

@seanPhill

Copy link
Copy Markdown
Collaborator

I should remind people that Ubuntu 24.04 or later is needed for this. I went out of my way on a previous build to support an old Ubuntu, but I won't this time.

Those with old Ubuntu will get a message like this:

./veild: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.32' not found (required by ./veild)
./veild: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.33' not found (required by ./veild)
./veild: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.34' not found (required by ./veild)
./veild: /lib/x86_64-linux-gnu/libstdc++.so.6: version `GLIBCXX_3.4.30' not found (required by ./veild)
./veild: /lib/x86_64-linux-gnu/libstdc++.so.6: version `GLIBCXX_3.4.29' not found (required by ./veild)
./veild: /lib/x86_64-linux-gnu/libstdc++.so.6: version `CXXABI_1.3.13' not found (required by ./veild)

It's best to just upgrade the host, from 20.04 LTS in the case I just tried.

$ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 20.04.6 LTS
Release:	20.04
Codename:	focal

@seanPhill

seanPhill commented Jan 28, 2026

Copy link
Copy Markdown
Collaborator

@blondfrogs On an Ubuntu server upgraded from 20.04 to 24.04 LTS I have a couple of testnet wallets, both with bloated 100-200 MB-ish wallet.dats. The one getting IPv4 peers is with the standard / automatic testnet settings, and the one set on IPv6 has a custom RPC port, username and password, and that all worked fine, and maybe this server is a bit undersized than some of my others, but they start and sync fine, but they are stopping after being unlocked, with the last line in the debug log being UnlockZerocoinWallet: cwallet set zerocoin masterseed to …
Any idea of what is going wrong?

$ ./veild -testnet
Segmentation fault (core dumped)

Restarting the released version 1.4.1.0.
Unlocked. Didn't crash. getwalletinfo … still running.

Second instance also running fine (on the released version), unlike the PR version.

(Too much time later … I revisited this, and I found an edge case causing that crash on the macOS GUI wallet, which is not related to the PR. Multiple wallets loaded in the Qt wallet, when enabling staking (the wallet conf file had staking=0, so it was not enabling immediately on unlock).)

I'll test this again on those testnet wallets, but the wallet.dat(s) have been replaced with unbloated/fresh ones since then. (The single wallet crash on unlocking/enabling staking might be related dependencies or something on the system that had just been upgraded from Ubuntu 20.04, and the old binary worked, while the new build didn't. No problem on macOS. Final testing soon!

@seanPhill

seanPhill commented May 12, 2026

Copy link
Copy Markdown
Collaborator

PR1053 wallets working fine on macOS (Sequoia) so long as the GUI unlocks for staking one wallet / not configured for multiple wallets (this is a rare case).

Ubuntu so far tested in CLI.

Ubuntu 22.04.5 LTS (GNU/Linux 5.15.0-130-generic x86_64) crashed when unlocked and staking is enabled!

Ubuntu 24.04.3 LTS (Linux 6.8.0-84-generic x86-64) crashed the same as the Ubuntu 22.04!

More wallets/systems remain to be tested.

WHEN unlocked and crashing, the last line in the debug.log is:
UnlockZerocoinWallet: cwallet set zerocoin masterseed to …
and veild disappears from the list in top -o %MEM

WHEN unlocked and not crashing, that line within less than a second is followed by:
operator(): Updating scriptpubkey for COutPoint(… 1)
operator(): Updating scriptpubkey for COutPoint(… 2)

and veild remains visible in the list in top -o %MEM

@seanPhill

seanPhill commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Testing again on Ubuntu 24.04.04 LTS.

Still crashing when a stakeable zerocoin is present and staking is enabled. (Imported my oldest 10K.)
Maybe the successful run on macOS was because I had no mature enough zerocoins for staking. (Yes, it crashed in macOS once it has a stakeable zerocoin.)

I turned all logging on and here are the final debug log lines up to the crash.
(Maybe this version can be released specifically for use for light wallet (eg. Zelcore) backend use, which is non-staking and doesn't handle zerocoins?)

$ tail -f ~/.veil/debug.log
Received a POST request for / from 127.0.0.1:42916
ThreadRPCServer method=walletpassphrase user=__cookie__
UnlockZerocoinWallet: cwallet set zerocoin masterseed to (…)
queue run of timer lockwallet() in 31557168 seconds (using HTTP)
ListMints : added 1 zerocoinmints from DB
ListMints: added 0 deterministic zerocoins from DB

Here is an extract from the macOS wallet's debug log. With all logging turned on I had to go back a bit to find the relevant lines before the final "ListMints" when it crashed.

UnlockZerocoinWallet: cwallet set zerocoin masterseed to (…)
queue run of timer lockwallet(TestingPR1053WalletFromUbuntu) in 31557168 seconds (using HTTP)
…
Flushing wallet.dat
Flushed wallet.dat 23ms
… 
ListMints : added 1 zerocoinmints from DB
ListMints: added 0 deterministic zerocoins from DB

@seanPhill seanPhill requested review from ohcee and seanPhill May 18, 2026 19:59

@seanPhill seanPhill left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please see my notes above, based on testing.

@ohcee ohcee left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Environment: M4 Mac Mini, macOS, mainnet synced (~block 3,848,700+), QT console. All tests run live on mainnet.


Verified

  • importstealthkeys — verified, round-trip confirmed with dumpprivkey
  • dumpprivkey stealth — verified, returns all four fields correctly
  • removewatchonlyaddress by address string — verified
  • getwatchonlystatus sync fix — verified
  • AddWatchOnlyAddress scan start fix — verified
  • anonwallet.cpp key lookup fixes — verified

Issues

Issue #1transactions_removed is never returned

Commit c43a625 explicitly claims "Returns count of transactions removed." It never appears in the response. EraseWatchOnlyAddressData reads txCount internally but the signature has no output parameter so the count is discarded before it reaches the RPC.

Issue #2 — Erase loop misses transactions at index 0

for (int i = 1; i <= txCount; i++)

The first transaction for a fresh address is written at index 0 via the current_count = -1 path in WriteWatchOnlyTx. The loop never erases it.


Incorrect Description

Commit 49affbf says "Added stealth key support to dumpwallet RPC." Only dumpprivkey was changed. Confirmed live dumpwallet output still contains the old note that stealth keys are not included.


Not Tested

PoS coinstake fix (7e90fcf) I do not have mature zerocoin coins available for staking on this setup so this could not be verified end to end.

@seanPhill

Copy link
Copy Markdown
Collaborator

Once the current bug fix PRs (committed today) have been run in master on several of my nodes for a while I will retry this PR1053 again, particularly for the seg-fault crashes when matured zerocoins are present with staking enabled.

(I've also just got Ubuntu 26.04 LTS now, so that's another thing to prove in testing.)

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.

4 participants