rpc, wallet: add abandoned field for all categories of transaction in ListTransaction #25158
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsNo conflicts as of last run. |
There was a problem hiding this comment.
Quick readover of the code as a break from seminar readings, but a key point would be to consider why this field was not returned other than for sent txns (and if concept ACKs, ensure there is appropriate test coverage in the tests that assert on this field. Quick grep:)
test/functional/feature_backwards_compatibility.py:197: assert not(txs[1]["abandoned"])
test/functional/feature_backwards_compatibility.py:200: assert txs[3]["abandoned"]
test/functional/wallet_abandonconflict.py:197: assert_equal(tx["abandoned"], False)
test/functional/wallet_abandonconflict.py:202: assert_equal(double_spend["abandoned"], False)
test/functional/wallet_abandonconflict.py:131: assert_equal(tx['abandoned'], True)
I couldn't find any resources about it, i'd like to understand more, btw Concept NACKs are welcome! I find #7739 which added this field but there is no explanation about why it is only in |
brunoerg
force-pushed
the
2022-05-abandoned-listtransactions
branch
from
6ef89bf
to
c3450ad
Compare
|
Force-pushed addressing @jonatack's comments |
brunoerg
force-pushed
the
2022-05-abandoned-listtransactions
branch
from
c3450ad
to
7e5a8a2
Compare
|
Are you still working on this? |
Yes. |
brunoerg
force-pushed
the
2022-05-abandoned-listtransactions
branch
from
7e5a8a2
to
efdb80a
Compare
|
ACK efdb80a I think it's reasonable to output this for all transactions, even if we decide that one type cannot be abandoned. A trivial case where this is useful are send-to-self transactions as these show up as individual send and receive transactions. |
|
You will have to modify the RPC docs for |
… in ListTransactions
…`listtransactions`
brunoerg
force-pushed
the
2022-05-abandoned-listtransactions
branch
from
efdb80a
to
0c52067
Compare
Nice find, thanks! Force-pushed modifying it. |
|
re-ACK 0c52067 |
…ies of transaction in ListTransaction 0c52067 doc: add release notes for `abandoned` field in `gettransaction` and `listtransactions` (brunoerg) a1aaa7f rpc, wallet: add `abandoned` field for all categories of transactions in ListTransactions (brunoerg) Pull request description: Fixes bitcoin#25130 ACKs for top commit: achow101: re-ACK 0c52067 Tree-SHA512: 1864460d76decab7898737c96517d722055eb8f81ca52248fe1035723258c6cd4a93251e06a86ecbbb0b0a80af1466b2c86fb142ace4ccb74cc40d5dc3967d7f
Fixes #25130