wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet #33268

pull achow101 wants to merge 5 commits into bitcoin:master from achow101:zero-value-from-me changing 8 files +182 −3
  1. achow101 commented at 10:28 pm on August 28, 2025: member

    One of the ways that the wallet would determine if a transaction was sent from the wallet was by checking if the total amount being spent by a transaction from outputs known to the wallet was greater than 0. This has worked fine until recently since there was no reason for 0-value outputs to be created. However, with ephemeral dust and P2A, it is possible to create standard 0-value outputs, and the wallet was not correctly identifying the spends of such outputs. This PR updates IsFromMe to only check whether the wallet knows any of the inputs, rather than checking the debit amount of a transaction.

    Additionally, a new functional test is added to test for this case, as well as a few other anchor output related scenarios. This also revealed a bug in sendall which would cause an assertion error when trying to spend all of the outputs in a wallet that has anchor outputs.

    Fixes #33265

  2. DrahtBot added the label Wallet on Aug 28, 2025
  3. DrahtBot commented at 10:28 pm on August 28, 2025: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33268.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK rkrux, enirox001, furszy
    Stale ACK jsarenik, w0xlt, kannapoix

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #31615 (validation: ensure assumevalid is always used during reindex by Eunovo)
    • #27865 (wallet: Track no-longer-spendable TXOs separately by achow101)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  4. achow101 force-pushed on Aug 28, 2025
  5. achow101 force-pushed on Aug 28, 2025
  6. DrahtBot added the label CI failed on Aug 28, 2025
  7. DrahtBot commented at 10:52 pm on August 28, 2025: contributor

    🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/49140364057 LLM reason (✨ experimental): Lint failure (py_lint) due to an unused import in test_wallet_anchor.py (CTransaction) flagged by ruff.

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  8. DrahtBot removed the label CI failed on Aug 29, 2025
  9. jsarenik commented at 8:00 am on August 29, 2025: none

    Tested ACK 7d1c3ce7f0f0. I have compiled it and ran the script from issue #33265 with no output other than the current UTC date. Works for me.

    Thanks!

  10. in src/wallet/wallet.cpp:1637 in e3a367156d outdated
    1633@@ -1634,7 +1634,11 @@ bool CWallet::IsMine(const COutPoint& outpoint) const
    1634 
    1635 bool CWallet::IsFromMe(const CTransaction& tx) const
    1636 {
    1637-    return (GetDebit(tx) > 0);
    1638+    LOCK(cs_wallet);
    


    instagibbs commented at 11:46 am on August 29, 2025:

    which is necessary for marking anchor outputs as spent.

    To be pedantic, it’s not about PayToAnchor(P2A), which can be any value, but any 0-value outputs, even if they weren’t ephemeral (say a miner mined it to dust your wallet).

    There are other legitimate 0-value output use-cases such as connector outputs, so let’s just call it 0-value dust?


    achow101 commented at 7:55 pm on August 29, 2025:
    Updated the commit message.
  11. jsarenik commented at 12:09 pm on August 29, 2025: none
    After it gets merged into master branch, I would recommend backporting it to current stable release branch which already contains the ephemeral outputs support (29.x).
  12. in test/functional/wallet_anchor.py:33 in 7d1c3ce7f0 outdated
    28+        self.num_nodes = 1
    29+
    30+    def skip_test_if_missing_module(self):
    31+        self.skip_if_no_wallet()
    32+
    33+    def test_anchor_listunspent(self):
    


    instagibbs commented at 1:03 pm on August 29, 2025:
    this subtest is covering 0-value outputs, not anchors per se. I’d rather keep the tests logically separated to not confused the concepts more than they already are :)

    achow101 commented at 7:55 pm on August 29, 2025:
    Clarified the test name
  13. instagibbs commented at 1:04 pm on August 29, 2025: member
    @jsarenik technically this issue could happen if miners mine 0-value outputs of any kind, but given that it’s now relay-possible, seems like a good idea to backport
  14. fanquake added the label Needs backport (29.x) on Aug 29, 2025
  15. achow101 force-pushed on Aug 29, 2025
  16. jsarenik commented at 6:53 pm on August 30, 2025: none
    Tested ACK de98ffb
  17. furszy commented at 5:42 pm on August 31, 2025: member
    q: is IsFromMe() method still relevant? It seems we could directly call IsMine() on the inputs, which is also cached now and should be slightly safer for migration.
  18. achow101 commented at 6:31 pm on August 31, 2025: member

    is IsFromMe() method still relevant?

    Yes, transactions that we make are still treated specially.

  19. furszy commented at 6:59 pm on August 31, 2025: member

    is IsFromMe() method still relevant?

    Yes, transactions that we make are still treated specially.

    Where?. IsFromMe() seems to be only ever called in AddToWalletIfInvolvingMe() and ApplyMigrationData() next to the IsMine() call. In both places we effectively treat it as is_mine = IsMine(outputs) || IsFromMe(inputs).

  20. achow101 commented at 7:30 pm on August 31, 2025: member

    Where?

    Hmm, I forgot that CachedTxIsFromMe does not use IsFromMe. Might need to change that in this PR as well.

  21. achow101 force-pushed on Sep 1, 2025
  22. achow101 commented at 10:50 pm on September 1, 2025: member

    Hmm, I forgot that CachedTxIsFromMe does not use IsFromMe. Might need to change that in this PR as well.

    I’ve pulled in 2 commits from #27865 which drop CachedTxIsFromMe and replace it with an IsFromMe check which is stored on disk.

    In both places we effectively treat it as is_mine = IsMine(outputs) || IsFromMe(inputs).

    While that’s true, moving IsFromMe into IsMine does change IsMine’s semantics in other places, so I’m not sure that that would necessarily be correct. I think it would be better to explore that in a different PR.

  23. achow101 force-pushed on Sep 1, 2025
  24. DrahtBot added the label CI failed on Sep 1, 2025
  25. DrahtBot commented at 11:06 pm on September 1, 2025: contributor

    🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/49358785571 LLM reason (✨ experimental): Lint Python code failure: unused variable fee in test_wallet_listtransactions.py (F841).

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  26. DrahtBot removed the label CI failed on Sep 2, 2025
  27. in test/functional/wallet_listtransactions.py:324 in e201a1e5a4 outdated
    316@@ -311,6 +317,45 @@ def test_op_return(self):
    317 
    318         assert 'address' not in op_ret_tx
    319 
    320+    def test_from_me_status_change(self):
    321+        self.log.info("Test gettransaction after changing a transaction's 'from me' status")
    322+        self.nodes[0].createwallet("fromme")
    323+        default_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
    324+        wallet = self.nodes[0].get_wallet_rpc("fromme")
    


    w0xlt commented at 6:49 pm on September 2, 2025:

    nit: perhaps it makes review easier.

    0        fromme_wallet = self.nodes[0].get_wallet_rpc("fromme")
    
  28. in src/wallet/wallet.cpp:1635 in e201a1e5a4 outdated
    1639@@ -1634,7 +1640,11 @@ bool CWallet::IsMine(const COutPoint& outpoint) const
    1640 
    1641 bool CWallet::IsFromMe(const CTransaction& tx) const
    


    w0xlt commented at 10:15 pm on September 2, 2025:

    nit:

    0bool CWallet::IsFromMe(const CTransaction& tx) const
    1{
    2    return std::any_of(tx.vin.begin(), tx.vin.end(),
    3        [this](const CTxIn& txin) { return WITH_LOCK(cs_wallet, return GetTXO(txin.prevout)); });
    4}
    
  29. achow101 force-pushed on Sep 2, 2025
  30. achow101 commented at 10:28 pm on September 2, 2025: member
    Didn’t really like m_from_me commits, so switched the implementation to just cache “from me” rather than also storing it on disk.
  31. w0xlt commented at 10:44 pm on September 2, 2025: contributor
  32. in test/functional/wallet_anchor.py:107 in 69eeb11bc5 outdated
    119+        assert_equal(utxos[0]["address"], ANCHOR_ADDRESS)
    120+        assert_equal(utxos[0]["amount"], 1)
    121+
    122+        assert_raises_rpc_error(-4, "Missing solving data for estimating transaction size", wallet.send, [{self.default_wallet.getnewaddress(): 0.9999}])
    123+        assert_raises_rpc_error(-4, "Error: Private keys are disabled for this wallet", wallet.sendtoaddress, self.default_wallet.getnewaddress(), 0.9999)
    124+        assert_raises_rpc_error(-4, "Unable to determine the size of the transaction, the wallet contains unsolvable descriptors", wallet.sendall, recipients=[self.default_wallet.getnewaddress()], inputs=utxos)
    


    kannapoix commented at 1:38 am on September 3, 2025:

    This error message might be confusing. A message like “Private keys are disabled for this wallet” would be more intuitive, similar to what sendtoaddress returns. https://github.com/bitcoin/bitcoin/blob/46369583f3a99d2b403349790f8b26dbc28de132/src/wallet/rpc/spend.cpp#L177-L179

    I’m not suggesting to remove the current error. It should still be raised when the wallet has private keys enabled but fails to determine the transaction size.


    rkrux commented at 12:48 pm on September 3, 2025:

    In https://github.com/bitcoin/bitcoin/commit/1cd57c67b10a771d2eb1c2324b241533c0b1f287 “test: Add a test for anchor outputs in the wallet”

    While passing the inputs in this RPC is fine, I feel another frequently used way of calling sendall RPC could be to not pass the inputs in the request, in which case the RPC throws Internal bug detected.

    0test_framework.authproxy.JSONRPCException: Internal bug detected: output.input_bytes > 0
    1wallet/rpc/spend.cpp:1524 (operator())
    2Bitcoin Core v29.99.0-69eeb11bc585
    3Please report this issue here: https://github.com/bitcoin/bitcoin/issues
    4 (-1)
    

    https://github.com/bitcoin/bitcoin/blob/ba0b4304eceeec52eb3ffd2e7f8bb831ba1ec278/src/wallet/rpc/spend.cpp#L1524

    One way to resolve (if the filtering is not done in AvailableCoins instead)

     0diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
     1index 45c85af9b6..d00547a897 100644
     2--- a/src/wallet/rpc/spend.cpp
     3+++ b/src/wallet/rpc/spend.cpp
     4@@ -1521,7 +1521,6 @@ RPCHelpMan sendall()
     5                 CoinFilterParams coins_params;
     6                 coins_params.min_amount = 0;
     7                 for (const COutput& output : AvailableCoins(*pwallet, &coin_control, fee_rate, coins_params).All()) {
     8-                    CHECK_NONFATAL(output.input_bytes > 0);
     9                     if (send_max && fee_rate.GetFee(output.input_bytes) > output.txout.nValue) {
    10                         continue;
    11                     }
    12diff --git a/test/functional/wallet_anchor.py b/test/functional/wallet_anchor.py
    13index 1d1435145a..49ef34c764 100755
    14--- a/test/functional/wallet_anchor.py
    15+++ b/test/functional/wallet_anchor.py
    16@@ -122,6 +122,7 @@ class WalletAnchorTest(BitcoinTestFramework):
    17         assert_raises_rpc_error(-4, "Missing solving data for estimating transaction size", wallet.send, [{self.default_wallet.getnewaddress(): 0.9999}])
    18         assert_raises_rpc_error(-4, "Error: Private keys are disabled for this wallet", wallet.sendtoaddress, self.default_wallet.getnewaddress(), 0.9999)
    19         assert_raises_rpc_error(-4, "Unable to determine the size of the transaction, the wallet contains unsolvable descriptors", wallet.sendall, recipients=[self.default_wallet.getnewaddress()], inputs=utxos)
    20+        assert_raises_rpc_error(-4, "Unable to determine the size of the transaction, the wallet contains unsolvable descriptors", wallet.sendall, recipients=[self.default_wallet.getnewaddress()])
    21 
    22     def run_test(self):
    23         self.default_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
    

    rkrux commented at 1:33 pm on September 3, 2025:
    I think this particular error is appropriately specific. The error in sendtoaddress, while easier to parse, is thrown because it also tries to sign the transaction, though at an earlier stage in the RPC flow. Perhaps this particular test can avoid testing for this different error because it is conspicuous.

    achow101 commented at 6:17 pm on September 3, 2025:
    Done
  33. kannapoix commented at 8:59 am on September 3, 2025: none

    ACK 69eeb11bc585e92a3b56811c3251f13dba05aa5f

    I have run the all functional tests and reviewed the code. I left a nit comment below.

  34. in test/functional/wallet_listtransactions.py:345 in 009b273df8 outdated
    340+            # and that there are other inputs belonging to only the sending wallet
    341+            send_res = default_wallet.send(outputs=[{wallet.getnewaddress(): 1.5}], inputs=utxos, add_inputs=True)
    342+            assert_equal(send_res["complete"], True)
    343+            txid = send_res["txid"]
    344+            self.nodes[0].syncwithvalidationinterfacequeue()
    345+            assert "fee" not in wallet.gettransaction(txid)
    


    rkrux commented at 11:21 am on September 3, 2025:

    In 009b273df8ad8a7470276a31f3cb900e480aa99b “test: Test wallet ‘from me’ status change”

    Probably can also assert on any detail entry being in send category after the descriptor is imported in the concerned wallet and not so before the import. Makes testing more explicit imho.

     0diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py
     1index a19a1a32b1..0c60d4839d 100755
     2--- a/test/functional/wallet_listtransactions.py
     3+++ b/test/functional/wallet_listtransactions.py
     4@@ -342,7 +342,9 @@ class ListTransactionsTest(BitcoinTestFramework):
     5             assert_equal(send_res["complete"], True)
     6             txid = send_res["txid"]
     7             self.nodes[0].syncwithvalidationinterfacequeue()
     8-            assert "fee" not in wallet.gettransaction(txid)
     9+            get_tx = wallet.gettransaction(txid)
    10+            assert "fee" not in get_tx
    11+            assert_equal(any(detail["category"] == "send" for detail in get_tx["details"]), False)
    12 
    13             if confirm:
    14                 self.generate(self.nodes[0], 1, sync_fun=self.no_op)
    15@@ -355,7 +357,9 @@ class ListTransactionsTest(BitcoinTestFramework):
    16             # TODO: We should check that the fee matches, but since the transaction spends inputs
    17             # not known to the wallet, it is incorrectly calculating the fee.
    18             # assert_equal(wallet.gettransaction(txid)["fee"], fee)
    19-            assert "fee" in wallet.gettransaction(txid)
    20+            get_tx = wallet.gettransaction(txid)
    21+            assert "fee" in get_tx
    22+            assert_equal(any(detail["category"] == "send" for detail in get_tx["details"]), True)
    23 
    24 if __name__ == '__main__':
    25     ListTransactionsTest(__file__).main()
    

    achow101 commented at 6:16 pm on September 3, 2025:
    Done as suggested
  35. in test/functional/wallet_listtransactions.py:353 in 009b273df8 outdated
    346+
    347+            if confirm:
    348+                self.generate(self.nodes[0], 1, sync_fun=self.no_op)
    349+                # Mock time forward and generate blocks so that the import does not rescan the transaction
    350+                self.nodes[0].setmocktime(int(time.time()) + MAX_FUTURE_BLOCK_TIME + 1)
    351+                self.generate(self.nodes[0], 10, sync_fun=self.no_op)
    


    rkrux commented at 11:41 am on September 3, 2025:

    In 009b273df8ad8a7470276a31f3cb900e480aa99b “test: Test wallet ‘from me’ status change”

    Are these necessary? The test passes without them even with a re-scan.

    0             if confirm:
    1                 self.generate(self.nodes[0], 1, sync_fun=self.no_op)
    2-                # Mock time forward and generate blocks so that the import does not rescan the transaction
    3-                self.nodes[0].setmocktime(int(time.time()) + MAX_FUTURE_BLOCK_TIME + 1)
    4-                self.generate(self.nodes[0], 10, sync_fun=self.no_op)
    

    achow101 commented at 5:51 pm on September 3, 2025:
    Yes, the specific failure we want to know about is when there is no rescan. Rescanning hides the problem.

    rkrux commented at 9:08 am on September 4, 2025:

    I don’t believe rescanning is skipped here at the moment. The following assertion passes, whereas putting it in the unexpected section throws.

     0diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py
     1index 714e659465..b590ed95d4 100755
     2--- a/test/functional/wallet_listtransactions.py
     3+++ b/test/functional/wallet_listtransactions.py
     4@@ -352,7 +352,8 @@ class ListTransactionsTest(BitcoinTestFramework):
     5                 self.nodes[0].setmocktime(int(time.time()) + MAX_FUTURE_BLOCK_TIME + 1)
     6                 self.generate(self.nodes[0], 10, sync_fun=self.no_op)
     7 
     8-            import_res = wallet.importdescriptors([{"desc": descriptor, "timestamp": "now"}])
     9+            with self.nodes[0].assert_debug_log(expected_msgs=["Rescanning last"]):
    10+                import_res = wallet.importdescriptors([{"desc": descriptor, "timestamp": "now"}])
    11             assert_equal(import_res[0]["success"], True)
    12             # TODO: We should check that the fee matches, but since the transaction spends inputs
    13             # not known to the wallet, it is incorrectly calculating the fee.
    

    rkrux commented at 9:19 am on September 4, 2025:
    0const [Params = <char[15], int>]] [fromme] RescanFromTime: Rescanning last 7 blocks
    

    Ah, I see now that last 7 blocks are rescanned and 10 blocks are generated previously - so the transaction is indeed not rescanned.

    Maybe the following diff for clarity:

    0- # Mock time forward and generate blocks so that the import does not rescan the transaction
    1+ # Mock time forward enough and generate blocks so that the import does not rescan the transaction 
    

    rkrux commented at 11:24 am on September 4, 2025:

    Interesting - took me some time to find out.

    I see that the transactions are marked dirty when the descriptor is imported that breaks the transaction caches. I confirmed this by commenting the following portion from the MarkDirty function after which the test fails:

    https://github.com/bitcoin/bitcoin/pull/33268/commits/113a4228229baedda2a730e097f2d59ad58a4b0d#diff-d41d68c5a65d67956c91b33ca86da7df1981d84a0b15b4a186deea566563fed5R344

    0m_cached_from_me = std::nullopt;
    

    I understand now why the test is written in this way to avoid the particular transaction from being rescanned again while ensuring the transaction caches are broken normally and the wallet TXOs are refreshed during descriptor import.

  36. in src/wallet/receive.cpp:1 in 69eeb11bc5


    rkrux commented at 11:42 am on September 3, 2025:

    In 69eeb11bc585e92a3b56811c3251f13dba05aa5f “wallet: Add m_cached_from_me to cache “from me” status”

    Maybe switch the order of the last 2 commits so that this commit also has coverage?


    achow101 commented at 6:02 pm on September 3, 2025:
    The test coverage for this commit is provided in the first commit of the PR. The point of that test is to ensure that behavior doesn’t change for commit.

    rkrux commented at 8:11 am on September 4, 2025:
    I see, I had missed the connection between the two commits.
  37. in test/functional/wallet_anchor.py:24 in 1cd57c67b1 outdated
    19+    assert_equal,
    20+    assert_raises_rpc_error,
    21+)
    22+from test_framework.wallet import MiniWallet
    23+
    24+ANCHOR_ADDRESS = "bcrt1pfeesnyr2tx"
    


    rkrux commented at 11:45 am on September 3, 2025:

    In 1cd57c67b10a771d2eb1c2324b241533c0b1f287 “test: Add a test for anchor outputs in the wallet”

    Nit: Probably can stay in the util file where PAY_TO_ANCHOR is defined.


    achow101 commented at 6:17 pm on September 3, 2025:
    Moved
  38. in test/functional/wallet_anchor.py:75 in 1cd57c67b1 outdated
    69+        utxos = wallet.listunspent()
    70+        assert_equal(len(utxos), 1)
    71+        assert_equal(utxos[0]["txid"], anchor_txid)
    72+        assert_equal(utxos[0]["address"], ANCHOR_ADDRESS)
    73+        assert_equal(utxos[0]["amount"], 0)
    74+        wallet.gettransaction(anchor_txid)
    


    rkrux commented at 11:56 am on September 3, 2025:

    In 1cd57c67b10a771d2eb1c2324b241533c0b1f287 “test: Add a test for anchor outputs in the wallet”

    supernit:

     0index 1d1435145a..a47f3c6b3c 100755
     1--- a/test/functional/wallet_anchor.py
     2+++ b/test/functional/wallet_anchor.py
     3@@ -71,13 +71,13 @@ class WalletAnchorTest(BitcoinTestFramework):
     4         assert_equal(utxos[0]["txid"], anchor_txid)
     5         assert_equal(utxos[0]["address"], ANCHOR_ADDRESS)
     6         assert_equal(utxos[0]["amount"], 0)
     7-        wallet.gettransaction(anchor_txid)
     8+        assert wallet.gettransaction(anchor_txid)
     9         assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", wallet.gettransaction, anchor_spend_txid)
    10 
    11         # Rescan the rest of the blockchain to see the anchor was spent
    12         wallet.rescanblockchain()
    13         assert_equal(wallet.listunspent(), [])
    14-        wallet.gettransaction(anchor_spend_txid)
    15+        assert wallet.gettransaction(anchor_spend_txid)
    

    achow101 commented at 5:53 pm on September 3, 2025:
    There’s nothing to assert, it either error and throws, or it succeeds and returns. The return value can never be false-y.

    rkrux commented at 9:14 am on September 4, 2025:

    Fair point, this suggestion was not correct.

    The intent of the suggestion was more for the cosmetic angle so that the reader doesn’t wonder why a standalone RPC is here without the presence of an assertion or usage of the returned value. Maybe there is an opportunity to introduce an assert_no_rpc_error helper here but need not be done now.

  39. in test/functional/wallet_anchor.py:82 in 1cd57c67b1 outdated
    77+        # Rescan the rest of the blockchain to see the anchor was spent
    78+        wallet.rescanblockchain()
    79+        assert_equal(wallet.listunspent(), [])
    80+        wallet.gettransaction(anchor_spend_txid)
    81+
    82+    def test_import_anchors(self):
    


    rkrux commented at 12:26 pm on September 3, 2025:

    In 1cd57c67b10a771d2eb1c2324b241533c0b1f287 “test: Add a test for anchor outputs in the wallet”

    There is a bit of duplication in all 3 tests. Maybe can remove this test altogether and put some of its functionality in the below test.

     0diff --git a/test/functional/wallet_anchor.py b/test/functional/wallet_anchor.py
     1index 1d1435145a..e6872deba0 100755
     2--- a/test/functional/wallet_anchor.py
     3+++ b/test/functional/wallet_anchor.py
     4@@ -79,40 +79,22 @@ class WalletAnchorTest(BitcoinTestFramework):
     5         assert_equal(wallet.listunspent(), [])
     6         wallet.gettransaction(anchor_spend_txid)
     7 
     8-    def test_import_anchors(self):
     9-        self.log.info("Test that descriptors for anchor outputs can only be imported to watchonly wallets")
    10-        self.nodes[0].createwallet(wallet_name="import_anchor_privkeys", blank=True)
    11-        wallet = self.nodes[0].get_wallet_rpc("import_anchor_privkeys")
    12-        import_res = wallet.importdescriptors([
    13-            {"desc": descsum_create(f"addr({ANCHOR_ADDRESS})"), "timestamp": "now"},
    14-            {"desc": descsum_create(f"raw({PAY_TO_ANCHOR.hex()})"), "timestamp": "now"}
    15-        ])
    16-        assert_equal(import_res[0]["success"], False)
    17-        assert_equal(import_res[1]["success"], False)
    18-
    19-        self.nodes[0].createwallet(wallet_name="import_anchor", disable_private_keys=True)
    20-        wallet = self.nodes[0].get_wallet_rpc("import_anchor")
    21-        import_res = wallet.importdescriptors([
    22-            {"desc": descsum_create(f"addr({ANCHOR_ADDRESS})"), "timestamp": "now"},
    23-            {"desc": descsum_create(f"raw({PAY_TO_ANCHOR.hex()})"), "timestamp": "now"}
    24-        ])
    25-        assert_equal(import_res[0]["success"], True)
    26-        assert_equal(import_res[1]["success"], True)
    27-
    28     def test_cannot_sign_anchors(self):
    29         self.log.info("Test that the wallet cannot spend anchor outputs")
    30-        self.nodes[0].createwallet(wallet_name="anchor_spend", disable_private_keys=True)
    31-        wallet = self.nodes[0].get_wallet_rpc("anchor_spend")
    32-        import_res = wallet.importdescriptors([
    33-            {"desc": descsum_create(f"addr({ANCHOR_ADDRESS})"), "timestamp": "now"},
    34-            {"desc": descsum_create(f"raw({PAY_TO_ANCHOR.hex()})"), "timestamp": "now"}
    35-        ])
    36-        assert_equal(import_res[0]["success"], True)
    37-        assert_equal(import_res[1]["success"], True)
    38+        for disable_private_keys in [False, True]:
    39+            self.nodes[0].createwallet(wallet_name=f"anchor_spend_{disable_private_keys}", disable_private_keys=disable_private_keys)
    40+            wallet = self.nodes[0].get_wallet_rpc(f"anchor_spend_{disable_private_keys}")
    41+            import_res = wallet.importdescriptors([
    42+                {"desc": descsum_create(f"addr({ANCHOR_ADDRESS})"), "timestamp": "now"},
    43+                {"desc": descsum_create(f"raw({PAY_TO_ANCHOR.hex()})"), "timestamp": "now"}
    44+            ])
    45+            assert_equal(import_res[0]["success"], disable_private_keys)
    46+            assert_equal(import_res[1]["success"], disable_private_keys)
    47 
    48         anchor_txid = self.default_wallet.sendtoaddress(ANCHOR_ADDRESS, 1)
    49         self.generate(self.nodes[0], 1)
    50 
    51+        wallet = self.nodes[0].get_wallet_rpc(f"anchor_spend_True")
    52         utxos = wallet.listunspent()
    53         assert_equal(len(utxos), 1)
    54         assert_equal(utxos[0]["txid"], anchor_txid)
    55@@ -126,7 +108,6 @@ class WalletAnchorTest(BitcoinTestFramework):
    56     def run_test(self):
    57         self.default_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
    58         self.test_0_value_anchor_listunspent()
    59-        self.test_import_anchors()
    60         self.test_cannot_sign_anchors()
    61 
    62 if __name__ == '__main__':
    

    achow101 commented at 6:17 pm on September 3, 2025:
    Done
  40. in src/wallet/receive.cpp:201 in 69eeb11bc5 outdated
    194@@ -195,7 +195,9 @@ void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
    195 
    196 bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx)
    197 {
    198-    return (CachedTxGetDebit(wallet, wtx, /*avoid_reuse=*/false) > 0);
    199+    if (wtx.m_cached_from_me.has_value()) return wtx.m_cached_from_me.value();
    200+    wtx.m_cached_from_me = wallet.IsFromMe(*wtx.tx);
    201+    return wtx.m_cached_from_me.value();
    


    rkrux commented at 1:08 pm on September 3, 2025:

    In https://github.com/bitcoin/bitcoin/commit/69eeb11bc585e92a3b56811c3251f13dba05aa5f “wallet: Add m_cached_from_me to cache “from me” status”

    nit to shorten:

     0diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp
     1index 4ed98c6d55..13a25b3632 100644
     2--- a/src/wallet/receive.cpp
     3+++ b/src/wallet/receive.cpp
     4@@ -195,8 +195,9 @@ void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
     5 
     6 bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx)
     7 {
     8-    if (wtx.m_cached_from_me.has_value()) return wtx.m_cached_from_me.value();
     9-    wtx.m_cached_from_me = wallet.IsFromMe(*wtx.tx);
    10+    if (!wtx.m_cached_from_me.has_value()) {
    11+        wtx.m_cached_from_me = wallet.IsFromMe(*wtx.tx);
    12+    }
    13     return wtx.m_cached_from_me.value();
    14 }
    

    achow101 commented at 6:18 pm on September 3, 2025:
    Done
  41. rkrux commented at 1:12 pm on September 3, 2025: contributor

    Concept ACK 69eeb11bc585e92a3b56811c3251f13dba05aa5f

    Agree with the intent. Nit in PR description because specifically IsFromMe is updated in the PR:

    0- One of the ways that the wallet would determine if a transaction belonged to the wallet was by checking
    1+ One of the ways that the wallet would determine if a transaction was sent from the wallet was by checking
    
  42. achow101 force-pushed on Sep 3, 2025
  43. achow101 force-pushed on Sep 3, 2025
  44. test: Test wallet 'from me' status change
    If something is imported into the wallet, it can change the 'from me'
    status of a transaction. This status is only visible through
    gettransaction's "fee" field which is only shown for transactions that
    are 'from me'.
    e76c2f7a41
  45. wallet: Determine IsFromMe by checking for TXOs of inputs
    Instead of checking whether the total amount of inputs known by the
    wallet is greater than 0, we should be checking for whether the input is
    known by the wallet. This enables us to determine whether a transaction
    spends an of output with an amount of 0, which is necessary for marking
    0-value dust outputs as spent.
    39a7dbdd27
  46. wallet: Throw an error in sendall if the tx size cannot be calculated c40dc822d7
  47. test: Add a test for anchor outputs in the wallet 609d265ebc
  48. wallet: Add m_cached_from_me to cache "from me" status
    m_cached_from_me is used to track whether a transaction is "from me", i.e. has
    any inputs which belong to the wallet. This is held in memory only in
    the same way that a transaction's balances are.
    113a422822
  49. achow101 force-pushed on Sep 3, 2025
  50. in test/functional/wallet_listtransactions.py:321 in e76c2f7a41 outdated
    316@@ -311,6 +317,49 @@ def test_op_return(self):
    317 
    318         assert 'address' not in op_ret_tx
    319 
    320+    def test_from_me_status_change(self):
    321+        self.log.info("Test gettransaction after changing a transaction's 'from me' status")
    


    rkrux commented at 11:25 am on September 4, 2025:

    In https://github.com/bitcoin/bitcoin/commit/009b273df8ad8a7470276a31f3cb900e480aa99b “test: Test wallet ‘from me’ status change”

    Nit if retouched:

    0- self.log.info("Test gettransaction after changing a transaction's 'from me' status")
    1+ self.log.info("Test gettransaction after changing a transaction's 'from me' status due to importing descriptor") 
    
  51. rkrux approved
  52. rkrux commented at 11:41 am on September 4, 2025: contributor

    lgtm ACK 113a4228229baedda2a730e097f2d59ad58a4b0d

    Generally, I find it easy to miss these Cached* functions because they lie in this wallet/receive.cpp file that I don’t find intuitive; wallet/transaction.cpp could be more intuitive because most of these functions update the CWallet object somehow - this PR need not do anything about it though.

    Ok with addition of m_cached_from_me within WalletTx in this PR but I find Cached* functions are too many to keep track of.

  53. DrahtBot requested review from kannapoix on Sep 4, 2025
  54. DrahtBot requested review from w0xlt on Sep 4, 2025
  55. enirox001 commented at 5:40 pm on September 8, 2025: contributor
    Tested ACK 113a422. Ran the full functional test suite including wallet_anchor.py; all tests passed. Fix for 0 value anchor detection and sendall size errors looks good. LGTM.
  56. furszy commented at 8:04 pm on September 11, 2025: member
    ACK 113a4228229baedda2a730e097f2d59ad58a4b0d
  57. fanquake merged this on Sep 12, 2025
  58. fanquake closed this on Sep 12, 2025

  59. instagibbs commented at 1:49 pm on September 12, 2025: member
    needs backport label for 30 as well?
  60. fanquake commented at 1:50 pm on September 12, 2025: member
    Putting it into #33356 shortly.
  61. fanquake referenced this in commit ad6c13e041 on Sep 12, 2025
  62. fanquake referenced this in commit d2be9a22d8 on Sep 12, 2025
  63. fanquake referenced this in commit b85dc7ed3a on Sep 12, 2025
  64. fanquake referenced this in commit bbb4e118f3 on Sep 12, 2025
  65. fanquake referenced this in commit 75026cddea on Sep 12, 2025
  66. fanquake commented at 2:00 pm on September 12, 2025: member
    Backported to 30.x in #33356.

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-10-10 18:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me