-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Bug-fix: listsinceblock: use fork point as reference for blocks in reorg'd chains #9516
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for including steps to reproduce. Those should be trivial to translate to our python test suite. Mind to add a commit with the new test? |
|
@MarcoFalke Will do! |
|
@MarcoFalke Done. Feedback welcome! |
qa/rpc-tests/listsinceblock.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: It is sufficient to only import what you need. (E.g. assert_equal)
Also, +x is missing on the permissions of this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx, feel free to squash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
8209dc1 to
8d78648
Compare
|
Good catch. utACK 8d78648 |
|
Why isn't this using LastCommonAncestor/FindFork or equivalent? I believe doing that would give precisely the correct behavior. (Aside: Good find. I had just assumed this was doing the LCA thing. :( ) |
|
@gmaxwell Nice! I didn't realize it existed. |
49812ee to
004208d
Compare
|
utACK. |
…h was provided for a chain that was not the main chain.
004208d to
297b2c2
Compare
297b2c2 to
7ba0a00
Compare
|
Looks like improvement, but I'm not sure it fixes all the issues: if a transaction was confirmed on the "since" block, and no longer is, we should include it in the results even if it did not change between the fork-point and tip. |
|
@luke-jr Yeah, a double spend of the same UTXO will result in no transaction showing up, which is definitely not ideal.. |
…en parameter is a reorg'd block 876e92b Testing: listsinceblock should display all transactions that were affected since the given block, including transactions that were removed due to a reorg. (Karl-Johan Alm) f999c46 listsinceblock: optionally find and list any transactions that were undone due to reorg when requesting a non-main chain block in a new 'removed' array. (Karl-Johan Alm) Pull request description: The following scenario will not notify the caller of the fact `tx0` has been dropped: 1. User 1 receives BTC in tx0 from utxo1 in block aa1. 2. User 2 receives BTC in tx1 from utxo1 (same) in block bb1 3. User 1 sees 2 confirmations at block aa3. 4. Reorg into bb chain. 5. User 1 asks `listsinceblock aa3` and does not see that tx0 is now invalidated. See `listsinceblock.py` commit for related test. The proposed fix is to iterate from the given block down to the fork point, and to check each transaction in the blocks against the wallet, in addition to including all transactions from the fork point to the active chain tip (the current behavior). Any transactions that were present will now also be listed in the `listsinceblock` output in a new `replaced` array. This operation may be a bit heavy but the circumstances (and perceived frequency of occurrence) warrant it, I believe. Example output: ```Python { 'transactions': [], 'replaced': [ { 'walletconflicts': [], 'vout': 1, 'account': '', 'timereceived': 1485234857, 'time': 1485234857, 'amount': '1.00000000', 'bip125-replaceable': 'unknown', 'trusted': False, 'category': 'receive', 'txid': 'ce673859a30dee1d2ebdb3c05f2eea7b1da54baf68f93bb8bfe37c5f09ed22ff', 'address': 'miqEt4kWp9zSizwGGuUWLAmxEcTW9bFUnQ', 'label': '', 'confirmations': -7 } ], 'lastblock': '7a388f27d09e3699102a4ebf81597d974fc4c72093eeaa02adffbbf7527f6715' } ``` I believe this addresses the comment by @luke-jr in #9516 (comment) but I could be wrong.. Tree-SHA512: 607b5dcaeccb9dc0d963d3de138c40490f3e923050b29821e6bd513d26beb587bddc748fbb194503fe618cfe34a6ed65d95e8d9c5764a882b6c5f976520cff35
…en parameter is a reorg'd block 876e92b Testing: listsinceblock should display all transactions that were affected since the given block, including transactions that were removed due to a reorg. (Karl-Johan Alm) f999c46 listsinceblock: optionally find and list any transactions that were undone due to reorg when requesting a non-main chain block in a new 'removed' array. (Karl-Johan Alm) Pull request description: The following scenario will not notify the caller of the fact `tx0` has been dropped: 1. User 1 receives BTC in tx0 from utxo1 in block aa1. 2. User 2 receives BTC in tx1 from utxo1 (same) in block bb1 3. User 1 sees 2 confirmations at block aa3. 4. Reorg into bb chain. 5. User 1 asks `listsinceblock aa3` and does not see that tx0 is now invalidated. See `listsinceblock.py` commit for related test. The proposed fix is to iterate from the given block down to the fork point, and to check each transaction in the blocks against the wallet, in addition to including all transactions from the fork point to the active chain tip (the current behavior). Any transactions that were present will now also be listed in the `listsinceblock` output in a new `replaced` array. This operation may be a bit heavy but the circumstances (and perceived frequency of occurrence) warrant it, I believe. Example output: ```Python { 'transactions': [], 'replaced': [ { 'walletconflicts': [], 'vout': 1, 'account': '', 'timereceived': 1485234857, 'time': 1485234857, 'amount': '1.00000000', 'bip125-replaceable': 'unknown', 'trusted': False, 'category': 'receive', 'txid': 'ce673859a30dee1d2ebdb3c05f2eea7b1da54baf68f93bb8bfe37c5f09ed22ff', 'address': 'miqEt4kWp9zSizwGGuUWLAmxEcTW9bFUnQ', 'label': '', 'confirmations': -7 } ], 'lastblock': '7a388f27d09e3699102a4ebf81597d974fc4c72093eeaa02adffbbf7527f6715' } ``` I believe this addresses the comment by @luke-jr in bitcoin/bitcoin#9516 (comment) but I could be wrong.. Tree-SHA512: 607b5dcaeccb9dc0d963d3de138c40490f3e923050b29821e6bd513d26beb587bddc748fbb194503fe618cfe34a6ed65d95e8d9c5764a882b6c5f976520cff35
…en parameter is a reorg'd block Summary: 876e92b Testing: listsinceblock should display all transactions that were affected since the given block, including transactions that were removed due to a reorg. (Karl-Johan Alm) f999c46 listsinceblock: optionally find and list any transactions that were undone due to reorg when requesting a non-main chain block in a new 'removed' array. (Karl-Johan Alm) Pull request description: The following scenario will not notify the caller of the fact `tx0` has been dropped: 1. User 1 receives BTC in tx0 from utxo1 in block aa1. 2. User 2 receives BTC in tx1 from utxo1 (same) in block bb1 3. User 1 sees 2 confirmations at block aa3. 4. Reorg into bb chain. 5. User 1 asks `listsinceblock aa3` and does not see that tx0 is now invalidated. See `listsinceblock.py` commit for related test. The proposed fix is to iterate from the given block down to the fork point, and to check each transaction in the blocks against the wallet, in addition to including all transactions from the fork point to the active chain tip (the current behavior). Any transactions that were present will now also be listed in the `listsinceblock` output in a new `replaced` array. This operation may be a bit heavy but the circumstances (and perceived frequency of occurrence) warrant it, I believe. Example output: ```Python { 'transactions': [], 'replaced': [ { 'walletconflicts': [], 'vout': 1, 'account': '', 'timereceived': 1485234857, 'time': 1485234857, 'amount': '1.00000000', 'bip125-replaceable': 'unknown', 'trusted': False, 'category': 'receive', 'txid': 'ce673859a30dee1d2ebdb3c05f2eea7b1da54baf68f93bb8bfe37c5f09ed22ff', 'address': 'miqEt4kWp9zSizwGGuUWLAmxEcTW9bFUnQ', 'label': '', 'confirmations': -7 } ], 'lastblock': '7a388f27d09e3699102a4ebf81597d974fc4c72093eeaa02adffbbf7527f6715' } ``` I believe this addresses the comment by @luke-jr in bitcoin/bitcoin#9516 (comment) but I could be wrong.. Tree-SHA512: 607b5dcaeccb9dc0d963d3de138c40490f3e923050b29821e6bd513d26beb587bddc748fbb194503fe618cfe34a6ed65d95e8d9c5764a882b6c5f976520cff35 Backport of Core PR 9622 https://github.com/bitcoin/bitcoin/pull/9622/files Completes T551 Test Plan: make check test_runner.py wallet_listsinceblock Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2689
…en parameter is a reorg'd block
Summary:
876e92b Testing: listsinceblock should display all transactions that were affected since the given block, including transactions that were removed due to a reorg. (Karl-Johan Alm)
f999c46 listsinceblock: optionally find and list any transactions that were undone due to reorg when requesting a non-main chain block in a new 'removed' array. (Karl-Johan Alm)
Pull request description:
The following scenario will not notify the caller of the fact `tx0` has been dropped:
1. User 1 receives BTC in tx0 from utxo1 in block aa1.
2. User 2 receives BTC in tx1 from utxo1 (same) in block bb1
3. User 1 sees 2 confirmations at block aa3.
4. Reorg into bb chain.
5. User 1 asks `listsinceblock aa3` and does not see that tx0 is now invalidated.
See `listsinceblock.py` commit for related test.
The proposed fix is to iterate from the given block down to the fork point, and to check each transaction in the blocks against the wallet, in addition to including all transactions from the fork point to the active chain tip (the current behavior). Any transactions that were present will now also be listed in the `listsinceblock` output in a new `replaced` array. This operation may be a bit heavy but the circumstances (and perceived frequency of occurrence) warrant it, I believe.
Example output:
```Python
{
'transactions': [],
'replaced': [
{
'walletconflicts': [],
'vout': 1,
'account': '',
'timereceived': 1485234857,
'time': 1485234857,
'amount': '1.00000000',
'bip125-replaceable': 'unknown',
'trusted': False,
'category': 'receive',
'txid': 'ce673859a30dee1d2ebdb3c05f2eea7b1da54baf68f93bb8bfe37c5f09ed22ff',
'address': 'miqEt4kWp9zSizwGGuUWLAmxEcTW9bFUnQ',
'label': '',
'confirmations': -7
}
],
'lastblock': '7a388f27d09e3699102a4ebf81597d974fc4c72093eeaa02adffbbf7527f6715'
}
```
I believe this addresses the comment by @luke-jr in bitcoin/bitcoin#9516 (comment) but I could be wrong..
Tree-SHA512: 607b5dcaeccb9dc0d963d3de138c40490f3e923050b29821e6bd513d26beb587bddc748fbb194503fe618cfe34a6ed65d95e8d9c5764a882b6c5f976520cff35
Backport of Core PR 9622
https://github.com/bitcoin/bitcoin/pull/9622/files
Completes T551
Test Plan:
make check
test_runner.py wallet_listsinceblock
Reviewers: deadalnix, Fabien, #bitcoin_abc
Reviewed By: Fabien, #bitcoin_abc
Subscribers: teamcity, schancel
Differential Revision: https://reviews.bitcoinabc.org/D2689
* test: Replace remaining sprintf with snprintf Summary: Use of `sprintf` is seen as a red flag as many of its uses are insecure. OpenBSD warns about it while compiling, and some modern platforms, e.g. [cloudlibc from cloudabi](https://github.com/NuxiNL/cloudlibc) don't even provide it anymore. Backport of core PR9867 Test Plan: make check Reviewers: #bitcoin_abc, deadalnix, jasonbcox Reviewed By: #bitcoin_abc, deadalnix, jasonbcox Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2697 * Merge #10783: [RPC] Various rpc argument fixes Summary: 4dc1915 check for null values in rpc args and handle appropriately (Gregory Sanders) 999ef20 importmulti options are optional (Gregory Sanders) a70d025 fixup some rpc param counting for rpc help (Gregory Sanders) Pull request description: Audited where named args will fail to use correct default values or may fail when additional optional arguments are added. Previously for these parameters, it was fine to omit them as positional arguments, but it would trigger UniValue runtime errors to set them to null, or to omit them while passing named parameters with greater positions (which would internally set earlier missing arguments to null). Now null values are treated the same as missing values so these errors do not occur. Included a few other small fixes while working on it. I didn't bother fixing account-based rpc calls. Tree-SHA512: 8baf781a35bd48de7878d4726850a580dab80323d3416c1c146b4fa9062f8a233c03f37e8ae3f3159e9d04a8f39c326627ca64c14e1cb7ce72538f934ab2ae1e Backport of Core PR 10783 https://github.com/bitcoin/bitcoin/pull/10783/files Completes T552 Test Plan: make check test_runner.py Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2688 * Merge #10775: nCheckDepth chain height fix Summary: d9d1bd3 nCheckDepth chain height fix (romanornr) Pull request description: ```` if (nCheckDepth <= 0) nCheckDepth = 1000000000; // suffices until the year 19000 if (nCheckDepth > chainActive.Height()) nCheckDepth = chainActive.Height(); ```` These lines confuse me. Correct me if I am wrong, but we can't check any more blocks than we have right? If someone requests <= 0 it get set it into some huge number and then immediately limit it to the chain height in the following statement. ```` if (nCheckDepth > chainActive.Height()) nCheckDepth = chainActive.Height(); ```` when using ````--checkblocks=Z```` When Z is ````0```` or any other negative number, it will check all blocks. I think it should be changed to this maybe. ```` if (nCheckDepth <= 0 || nCheckDepth > chainActive.Height()) nCheckDepth = chainActive.Height(); ```` Which gets rid of that huge number which is confusing for any other altcoins that have a different block time. Tree-SHA512: 8ee0ae5f33b399fa74dc16926709694ccfe1fc8a043cba2f5d00884220ac1b9b13f2df4588041f4133be634e5c7b14f4eebe24294028dafe91581a97dbe627f3 Backport of Core PR 10775 https://github.com/bitcoin/bitcoin/pull/10775/files Test Plan: ninja check test_runner.py Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2691 * Merge #9622: [rpc] listsinceblock should include lost transactions when parameter is a reorg'd block Summary: 876e92b Testing: listsinceblock should display all transactions that were affected since the given block, including transactions that were removed due to a reorg. (Karl-Johan Alm) f999c46 listsinceblock: optionally find and list any transactions that were undone due to reorg when requesting a non-main chain block in a new 'removed' array. (Karl-Johan Alm) Pull request description: The following scenario will not notify the caller of the fact `tx0` has been dropped: 1. User 1 receives BTC in tx0 from utxo1 in block aa1. 2. User 2 receives BTC in tx1 from utxo1 (same) in block bb1 3. User 1 sees 2 confirmations at block aa3. 4. Reorg into bb chain. 5. User 1 asks `listsinceblock aa3` and does not see that tx0 is now invalidated. See `listsinceblock.py` commit for related test. The proposed fix is to iterate from the given block down to the fork point, and to check each transaction in the blocks against the wallet, in addition to including all transactions from the fork point to the active chain tip (the current behavior). Any transactions that were present will now also be listed in the `listsinceblock` output in a new `replaced` array. This operation may be a bit heavy but the circumstances (and perceived frequency of occurrence) warrant it, I believe. Example output: ```Python { 'transactions': [], 'replaced': [ { 'walletconflicts': [], 'vout': 1, 'account': '', 'timereceived': 1485234857, 'time': 1485234857, 'amount': '1.00000000', 'bip125-replaceable': 'unknown', 'trusted': False, 'category': 'receive', 'txid': 'ce673859a30dee1d2ebdb3c05f2eea7b1da54baf68f93bb8bfe37c5f09ed22ff', 'address': 'miqEt4kWp9zSizwGGuUWLAmxEcTW9bFUnQ', 'label': '', 'confirmations': -7 } ], 'lastblock': '7a388f27d09e3699102a4ebf81597d974fc4c72093eeaa02adffbbf7527f6715' } ``` I believe this addresses the comment by @luke-jr in bitcoin/bitcoin#9516 (comment) but I could be wrong.. Tree-SHA512: 607b5dcaeccb9dc0d963d3de138c40490f3e923050b29821e6bd513d26beb587bddc748fbb194503fe618cfe34a6ed65d95e8d9c5764a882b6c5f976520cff35 Backport of Core PR 9622 https://github.com/bitcoin/bitcoin/pull/9622/files Completes T551 Test Plan: make check test_runner.py wallet_listsinceblock Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2689 * Merge #11039: Avoid second mapWallet lookup Summary: 8f2f1e0 wallet: Avoid second mapWallet lookup (João Barbosa) Pull request description: All calls to `mapWallet.count()` have the intent to detect if a `txid` exists and most are followed by a second lookup to retrieve the `CWalletTx`. This PR replaces all `mapWallet.count()` calls with `mapWallet.find()` to avoid the second lookup. Tree-SHA512: 96b7de7f5520ebf789a1aec1949a4e9c74e13683869cee012f717e5be8e51097d068e2347a36e89097c9a89f1ed1a1529db71760dac9b572e36a3e9ac1155f29 Backport of Core PR 11039 https://github.com/bitcoin/bitcoin/pull/11039/files Completes T550 Depends on D2689 Test Plan: ninja check test_runner.py Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2690 * Merge #11027: [RPC] Only return hex field once in getrawtransaction Summary: 6bbdafc Pass serialization flags and whether to include hex to TxToUniv (Andrew Chow) e029c6e Only return hex field once in getrawtransaction (Andrew Chow) Pull request description: The hex is already returned in `TxToUniv()`, no need to give it out a second time in getrawtransaction itself. Tree-SHA512: 270289f2d6dea37f51f5a42db3dae5debdbe83c6b504fccfd3391588da986ed474592c6655d522dc51022d4b08fa90ed1ebb249afe036309f95adfe3652cb262 Backport of Core PR 11027 bitcoin/bitcoin#11027 Completes T564 Test Plan: ninja check test_runner.py Run `bitcoin-cli <txid> 1` and ensure that there is a single top-level `"hex"` field in the returned JSON. Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2709 * Merge #11565: Make listsinceblock refuse unknown block hash Summary: 659b206 Make listsinceblock refuse unknown block hash (Russell Yanofsky) Pull request description: Change suggested by @theuni who noticed listsinceblock would ignore invalid block hashes causing it to return a completely unfiltered list of transactions. Tree-SHA512: 3c8fb160265780d1334e856e853ab48e2e18372b8f1fc71ae480c3f45317048cc1fee0055d5c58031981a91b9c2bdbeb8e49a889d04ecba61729ce8109f2ce3f Backport of Core PR 11565 https://github.com/bitcoin/bitcoin/pull/11565/files Completes T563 Test Plan: ninja check wallet_listsinceblock.py Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2703 * Merge #11618: rpc: Lock cs_main in blockToJSON/blockheaderToJSON Summary: a9b6ba0b7 Add missing cs_main locks when calling blockToJSON/blockheaderToJSON (practicalswift) Pull request description: `blockToJSON(...)` and `blockheaderToJSON(...)` read the variable `chainActive` which requires holding the mutex `cs_main`. So does `GetDifficulty(...)`. Tree-SHA512: bfb94f5e3238accbf6a4daddde49d53f1891c38ae9b07e25b3098c485747159258f64bb66a50e147b32beac601de89d9d04ff717b6c4f1460d329c90a53d3333 Backport of Core PR 11618 https://github.com/bitcoin/bitcoin/pull/11618/files Test Plan: make check test_runner.py bitcoin-cli getblock 000000000933ea01ad0ee984209779baaec3ced90fa3f408719526f8d77f4943 Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2684 * Remove redundant locks Summary: * SetAddressBook(...) is locking cs_wallet internally * DelAddressBook(...) is locking cs_wallet internally Backport of PR11733 bitcoin/bitcoin#11733 Test Plan: make check test_runner.py Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2700 * Merge #12327: [gui] Defer coin control instancing Summary: 6558f8acc [gui] Defer coin control instancing (João Barbosa) Pull request description: Defer the GUI coin control instancing so that argument processing is taken into account for the default coin control values. Fixes #12312 Tree-SHA512: ecda28b94f4709319e9484b01afe763c7c3569097d2afb89db79da8a195c46d20ea77166df7edce0c8ab77627b295def01c072148714503436d27675d5e75d99 Backport of Core PR 12327 https://github.com/bitcoin/bitcoin/pull/12327/files Completes T556 Test Plan: make check test_runner.py Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2693 * [LINTER] Improve check-doc regex Summary: Improve the regex: - Use Perl type regex in grep to remove duplication. Previously there was 2 alsmost identical regex, one posix compliant for egrep and one Perl compliant for python re. - Exclude the test folder from the source path rather than subtracting its matching output This improvements make the `-h` now detectable; it is added to the undocumented exceptions. Inspired from core PR12820. Note that GNU `grep` is used instead of `git grep` to allow for multiline regex (required due to our code format). Test Plan: Should return no error: arc lint ./test/lint/check-doc.py There should be 1 more used argument than before this patch (`-h`), otherwise the output should be identical. Reviewers: #bitcoin_abc, deadalnix, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2694 * [LINTER] Fix check doc incompatibility with BSD grep Summary: BSD grep on OSX and FreeBSD does not provide the same options as GNU grep which prevent using them for multiline search. This diff uses only python regex to avoid this incompatibility. As a bonus, it runs about 3x faster. Depends on D2694 Test Plan: Should return no error: arc lint In `src/init.cpp` delete the line 1279: ``` logger.m_print_to_console = gArgs.GetBoolArg("-printtoconsole", false); ``` Then run `arc lint` and check the linter outputs an Unknown argument error: '-printtoconsole' is documented but not used' Restore the previously deleted line in `src/init.cpp` and now delete the lines 792 to 794: ``` strUsage += HelpMessageOpt( "-printtoconsole", _("Send trace/debug info to console instead of debug.log file")); ``` Then run `arc lint` and check the linter outputs an Undocumented argument error: '-printtoconsole' is undocumented' Reviewers: #bitcoin_abc, deadalnix, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2695 * Backport RetFormat enum class Summary: Partial backport of Core PR 10742 (RetFormat only) https://github.com/bitcoin/bitcoin/pull/10742/files Progess towards T549 Test Plan: ninja check Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2712 * Backport BlockSource enum class Summary: Partial backport of Core PR 10742 (BlockSource only) https://github.com/bitcoin/bitcoin/pull/10742/files Progress towards T549 Test Plan: ninja check Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2713 * Backport HelpMessageMode enum class Summary: Partial backport of Core PR 10742 (HelpMessageMode only) https://github.com/bitcoin/bitcoin/pull/10742/files Progress towards T549 Test Plan: ninja check Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2714 * Merge #8665: Assert all the things! Summary: 4d51e9b Assert ConnectBlock block and pIndex are the same block (NicolasDorier) 972714c pow: GetNextWorkRequired never called with NULL pindexLast (Daniel Cousens) cc44c8f ContextualCheckBlockHeader should never have pindexPrev to NULL (NicolasDorier) Tree-SHA512: 7cc568bf9417267c335f21ec3d1505b26e56e5b3d5f4d3dbb555279489800aaa65a3bcd7bc376e274dd102912aec16ddbb18de2e2060b2667b41eb979cd9321e Backport of Core PR 8665 https://github.com/bitcoin/bitcoin/pull/8665/files Test Plan: make check test_runner.py Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2722 * Merge #11028: Avoid masking of difficulty adjustment errors by checkpoints Summary: 85c82b5 Avoid masking of difficulty adjustment errors by checkpoints (Pieter Wuille) Pull request description: Currently difficulty adjustment violations are not reported for chains that branch off before the last checkpoint. Change this by moving the checkpoint check after the difficulty check. Tree-SHA512: 33666f2c3459151b28c42041a463779e6df18f61d3dd5b1879a0af4e5b199ef74d1e33e06af68bebfdfb211569ad5fb56556bfebe9d63b5688d910ea211b839a Backport of Core PR 11028 https://github.com/bitcoin/bitcoin/pull/11028/files Depends on D2722 Test Plan: ``` make check test_runner.py ``` Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2723 * Avoid slow transaction search with txindex enabled Summary: Backport of PR11529: bitcoin/bitcoin#11529 Completes T565 Test Plan: make check test_runner.py Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: Fabien, O1 Bitcoin ABC, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2710 * [secp256k1] fix java secp256k1 test Summary: fix java secp256k1 test Test Plan: to run the java secp256k1 tests (including this one): ``` $ cd src/secp256k1 $ ./autogen.sh $ ./configure --enable-jni --enable-experimental --enable-module-ecdh $ make check-java ``` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien, jasonbcox Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien, jasonbcox Subscribers: jasonbcox, teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2685 * Backport FlushStateMode enum class Summary: Partial backport of Core PR 10742 (FlushStateMode only) https://github.com/bitcoin/bitcoin/pull/10742/files Progress towards T549 Test Plan: ninja check Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2716 * Allow for running secp256k1 java build/tests out of tree Summary: As per title Test Plan: mkdir -p src/secp256k1/build cd src/secp256k1 mkdir -p src/java/guava wget https://search.maven.org/remotecontent?filepath=com/google/guava/guava/18.0/guava-18.0.jar -O src/java/guava/guava-18.0.jar ./autogen.sh cd build ../configure --enable-jni --enable-experimental --enable-module-ecdh make check-java (`make check-java` may pass or fail depending of D2685) Use `make clean` to remove the .class files generated by `javac`. Reviewers: #bitcoin_abc, deadalnix, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2717 * Backport VerifyResult enum class Summary: Partial backport of Core PR 10742 (VerifyResult only) https://github.com/bitcoin/bitcoin/pull/10742/files Progress towards T549 Test Plan: ninja check Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2718 * Remove unused seeder/compat.h Summary: This file is no longer used in the seeder since it is included in the main tree (src/compat.h is used). Test Plan: ./autogen.sh mkdir build && cd build ../configure make ./src/bitcoin-seeder Ensure the software runs as expected cd .. mkdir buildcmake && cd buildcmake cmake -GNinja .. ninja ./src/bitcoin-seeder Ensure the software runs as expected Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2728 * [secp256k1] remove guava dep Summary: remove overkill depencency to guava Test Plan: execute previous tests successfully the JNI part can be built and tested by simply doing ``` $ ./autogen.sh $ ./configure --enable-jni --enable-experimental --enable-module-ecdh $ make check-java ``` Once https://reviews.bitcoinabc.org/D2686 is passed we can do this automatically Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien Subscribers: Fabien, teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2681 * [secp256k1] remove unused byte array Summary: remove unused bytearray in verify Test Plan: re run tests Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Subscribers: schancel Differential Revision: https://reviews.bitcoinabc.org/D2734 * Fixup merge issue * Remove seeder.h from make
…en parameter is a reorg'd block
Summary:
876e92b Testing: listsinceblock should display all transactions that were affected since the given block, including transactions that were removed due to a reorg. (Karl-Johan Alm)
f999c46 listsinceblock: optionally find and list any transactions that were undone due to reorg when requesting a non-main chain block in a new 'removed' array. (Karl-Johan Alm)
Pull request description:
The following scenario will not notify the caller of the fact `tx0` has been dropped:
1. User 1 receives BTC in tx0 from utxo1 in block aa1.
2. User 2 receives BTC in tx1 from utxo1 (same) in block bb1
3. User 1 sees 2 confirmations at block aa3.
4. Reorg into bb chain.
5. User 1 asks `listsinceblock aa3` and does not see that tx0 is now invalidated.
See `listsinceblock.py` commit for related test.
The proposed fix is to iterate from the given block down to the fork point, and to check each transaction in the blocks against the wallet, in addition to including all transactions from the fork point to the active chain tip (the current behavior). Any transactions that were present will now also be listed in the `listsinceblock` output in a new `replaced` array. This operation may be a bit heavy but the circumstances (and perceived frequency of occurrence) warrant it, I believe.
Example output:
```Python
{
'transactions': [],
'replaced': [
{
'walletconflicts': [],
'vout': 1,
'account': '',
'timereceived': 1485234857,
'time': 1485234857,
'amount': '1.00000000',
'bip125-replaceable': 'unknown',
'trusted': False,
'category': 'receive',
'txid': 'ce673859a30dee1d2ebdb3c05f2eea7b1da54baf68f93bb8bfe37c5f09ed22ff',
'address': 'miqEt4kWp9zSizwGGuUWLAmxEcTW9bFUnQ',
'label': '',
'confirmations': -7
}
],
'lastblock': '7a388f27d09e3699102a4ebf81597d974fc4c72093eeaa02adffbbf7527f6715'
}
```
I believe this addresses the comment by @luke-jr in bitcoin/bitcoin#9516 (comment) but I could be wrong..
Tree-SHA512: 607b5dcaeccb9dc0d963d3de138c40490f3e923050b29821e6bd513d26beb587bddc748fbb194503fe618cfe34a6ed65d95e8d9c5764a882b6c5f976520cff35
Backport of Core PR 9622
https://github.com/bitcoin/bitcoin/pull/9622/files
Completes T551
Test Plan:
make check
test_runner.py wallet_listsinceblock
Reviewers: deadalnix, Fabien, #bitcoin_abc
Reviewed By: Fabien, #bitcoin_abc
Subscribers: teamcity, schancel
Differential Revision: https://reviews.bitcoinabc.org/D2689
* Merge #9622: [rpc] listsinceblock should include lost transactions when parameter is a reorg'd block
Summary:
876e92b Testing: listsinceblock should display all transactions that were affected since the given block, including transactions that were removed due to a reorg. (Karl-Johan Alm)
f999c46 listsinceblock: optionally find and list any transactions that were undone due to reorg when requesting a non-main chain block in a new 'removed' array. (Karl-Johan Alm)
Pull request description:
The following scenario will not notify the caller of the fact `tx0` has been dropped:
1. User 1 receives BTC in tx0 from utxo1 in block aa1.
2. User 2 receives BTC in tx1 from utxo1 (same) in block bb1
3. User 1 sees 2 confirmations at block aa3.
4. Reorg into bb chain.
5. User 1 asks `listsinceblock aa3` and does not see that tx0 is now invalidated.
See `listsinceblock.py` commit for related test.
The proposed fix is to iterate from the given block down to the fork point, and to check each transaction in the blocks against the wallet, in addition to including all transactions from the fork point to the active chain tip (the current behavior). Any transactions that were present will now also be listed in the `listsinceblock` output in a new `replaced` array. This operation may be a bit heavy but the circumstances (and perceived frequency of occurrence) warrant it, I believe.
Example output:
```Python
{
'transactions': [],
'replaced': [
{
'walletconflicts': [],
'vout': 1,
'account': '',
'timereceived': 1485234857,
'time': 1485234857,
'amount': '1.00000000',
'bip125-replaceable': 'unknown',
'trusted': False,
'category': 'receive',
'txid': 'ce673859a30dee1d2ebdb3c05f2eea7b1da54baf68f93bb8bfe37c5f09ed22ff',
'address': 'miqEt4kWp9zSizwGGuUWLAmxEcTW9bFUnQ',
'label': '',
'confirmations': -7
}
],
'lastblock': '7a388f27d09e3699102a4ebf81597d974fc4c72093eeaa02adffbbf7527f6715'
}
```
I believe this addresses the comment by @luke-jr in bitcoin/bitcoin#9516 (comment) but I could be wrong..
Tree-SHA512: 607b5dcaeccb9dc0d963d3de138c40490f3e923050b29821e6bd513d26beb587bddc748fbb194503fe618cfe34a6ed65d95e8d9c5764a882b6c5f976520cff35
Backport of Core PR 9622
https://github.com/bitcoin/bitcoin/pull/9622/files
Completes T551
Test Plan:
make check
test_runner.py wallet_listsinceblock
Reviewers: deadalnix, Fabien, #bitcoin_abc
Reviewed By: Fabien, #bitcoin_abc
Subscribers: teamcity, schancel
Differential Revision: https://reviews.bitcoinabc.org/D2689
* Fix unlocked_until in getwalletinfo rpc
Summary: Getwalletinfo returned an uninitialized value for unlocked_until before the first walletpassphrase call, even though the wallet was locked and it should return 0.
Test Plan:
make check
./wallet_encryption.py
Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien
Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien
Subscribers: teamcity, schancel
Differential Revision: https://reviews.bitcoinabc.org/D2633
* Remove nonnull warning when calling secp256k1_schnorr_sign with NULL noncefp
Summary: This warning was hidden under normal conditions because SECP256K1_ARG_NONNULL is bypassed when SECP256K1_BUILD is set. A NULL noncefp means the default nonce function is used.
Test Plan: make check
Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix
Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix
Subscribers: schancel
Differential Revision: https://reviews.bitcoinabc.org/D2747
* [Part 2 of 5] Add a CChainState class to clarify internal interfaces
Summary:
Move block writing out of AcceptBlock
Backport of Core PR 10279 commit e104f0f
bitcoin/bitcoin@e104f0f
Depends on D1968
Progress towards T572
Test Plan:
make check
test_runner.py
Reviewers: #bitcoin_abc, schancel, Fabien
Reviewed By: #bitcoin_abc, Fabien
Subscribers: jasonbcox, teamcity
Differential Revision: https://reviews.bitcoinabc.org/D1969
* [secp256k1] add schnorr verify jni binding
Summary: schnorr jni binding that will allow jvm support
Test Plan:
use the test vector data from
https://github.com/sipa/bips/blob/bip-schnorr/bip-schnorr/test-vectors.csv
Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, jasonbcox
Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, jasonbcox
Subscribers: jasonbcox, schancel
Differential Revision: https://reviews.bitcoinabc.org/D2745
* Merge #10858: [RPC] Add "warnings" field to getblockchaininfo and unify "warnings" field in get*info RPCs
Summary:
395cef7 Change getmininginfo errors field to warnings (Andrew Chow)
8502b20 Unify help text for GetWarnings output in get*info RPCs (Andrew Chow)
f77f0e4 Add warnings field to getblockchaininfo (Andrew Chow)
Pull request description:
The `getblockchaininfo` output does not contain the `warnings` field which the `getinfo`, `getmininginfo`, and `getnetworkinfo` RPCs have. It should have it as the errors pertain to the blockchain. This PR adds that field.
This PR also unifies the help text for the `warnings` field and its output position so that all of the `get*info` commands are consistent.
`getnetworkinfo`'s `errors` field is named `warnings`. I did not change this even though it is inconsistent since this naming has been in use for a long time.
Tree-SHA512: 385ab6acfee67fc8816f4d51ab2bd7a623264c7973906dfbab0a171f199e9db16fde19093a5bc3dfbdd4ff5f19d2186b646eb6b3bae0a4d7c9add43650a4a9d9
Backport of PR10858
bitcoin/bitcoin@9a8e916?diff=unified#diff-a0c8f511d90e83aa9b5857e819ced344R1165
Completes T567
Test Plan:
make check
test_runner.py
getblockchaininfo should also produce a "warnings" field
getmininginfo should have a "warnings" field in place of its "errors" field during regular use
getmininginfo should behave as previously (i.e. have an "errors" field) when running bitcoind -deprecatedrpc=getmininginfo
Reviewers: deadalnix, jasonbcox, Fabien, O1 Bitcoin ABC, #bitcoin_abc
Reviewed By: jasonbcox, Fabien, O1 Bitcoin ABC, #bitcoin_abc
Subscribers: schancel
Differential Revision: https://reviews.bitcoinabc.org/D2730
* remove unused fnoncriticalerrors variable from cwalletdb::findwallettx
Summary:
remove unused fnoncriticalerrors variable from cwalletdb::findwallettx
Backport of Core PR11923
https://github.com/bitcoin/bitcoin/pull/11923/files
Completes T570
Test Plan:
make check
test_runner.py
bitcoin-qt
Reviewers: Fabien, jasonbcox, deadalnix, O1 Bitcoin ABC, #bitcoin_abc
Reviewed By: deadalnix, O1 Bitcoin ABC, #bitcoin_abc
Subscribers: schancel
Differential Revision: https://reviews.bitcoinabc.org/D2742
* Fix comment about s in schnorr sigs
Summary: s/order/n/ for consistency.
Test Plan: None
Reviewers: markblundeberg, O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien, jasonbcox
Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien, jasonbcox
Subscribers: schancel
Differential Revision: https://reviews.bitcoinabc.org/D2741
* [Part 3 of 5] Add a CChainState class to clarify internal interfaces
Summary:
Make DisconnectBlock unaware of where undo data resides on disk
Backport of core PR 10279 Commit 93a34cf.
Depends on D1969
Test Plan:
make VERBOSE=1 check && ./test/functional/test_runner.py
Reviewers: #bitcoin_abc, O1 Bitcoin ABC, jasonbcox
Reviewed By: #bitcoin_abc, O1 Bitcoin ABC, jasonbcox
Subscribers: teamcity
Differential Revision: https://reviews.bitcoinabc.org/D1970
* Lint everything
Summary: As per title
Test Plan:
make check
./test/functional/test_runner.py
Reviewers: #bitcoin_abc, Fabien
Reviewed By: #bitcoin_abc, Fabien
Subscribers: schancel
Differential Revision: https://reviews.bitcoinabc.org/D2758
* Merge #10275: [rpc] Allow fetching tx directly from specified block in getrawtransaction
Summary:
434526a [test] Add tests for getrawtransaction with block hash. (Karl-Johan Alm)
b167951 [rpc] Allow getrawtransaction to take optional blockhash to fetch transaction from a block directly. (Karl-Johan Alm)
a5f5a2c [rpc] Fix fVerbose parsing (remove excess if cases). (Karl-Johan Alm)
Pull request description:
[Reviewer hint: use [?w=1](https://github.com/bitcoin/bitcoin/pull/10275/files?w=1) to avoid seeing a bunch of indentation changes.]
Presuming a user knows the block hash of the block containing a given transaction, this PR allows them to fetch the raw transaction, even without `-txindex`. It also enables support for getting transactions that are in orphaned blocks.
Note that supplying a block hash will override mempool and txindex support in `GetTransaction`. The rationale behind this is that a transaction may be in multiple places (orphaned blocks) and if the user supplies an explicit block hash it should be adhered to.
```Bash
$ # a41.. is a tx inside an orphan block ..3c6f.. -- first try getting it normally
$ ./bitcoin-cli getrawtransaction a41e66ee1341aa9fb9475b98cfdc1fe1261faa56c0a49254f33065ec90f7cd79 1
error code: -5
error message:
No such mempool transaction. Use -txindex to enable blockchain transaction queries. Use gettransaction for wallet transactions.
$ # now try with block hash
$ ./bitcoin-cli getrawtransaction a41e66ee1341aa9fb9475b98cfdc1fe1261faa56c0a49254f33065ec90f7cd79 1 0000000000000000003c6fe479122bfa4a9187493937af1734e1e5cd9f198ec7
{
"hex": "01000000014e7e81144e42f6d65550e59b715d470c9301fd7ac189[...]90488ac00000000",
"inMainChain": false,
"txid": "a41e66ee1341aa9fb9475b98cfdc1fe1261faa56c0a49254f33065ec90f7cd79",
"hash": "a41e66ee1341aa9fb9475b98cfdc1fe1261faa56c0a49254f33065ec90f7cd79",
"size": 225,
[...]
}
$ # another tx 6c66... in block 462000
$ ./bitcoin-cli getrawtransaction 6c66b98191e9d6cc671f6817142152ebf6c5cab2ef008397b5a71ac13255a735 1 00000000000000000217f2c12922e321f6d4aa933ce88005a9a493c503054a40
{
"hex": "0200000004d157[...]88acaf0c0700",
"inMainChain": true,
"txid": "6c66b98191e9d6cc671f6817142152ebf6c5cab2ef008397b5a71ac13255a735",
"hash": "6c66b98191e9d6cc671f6817142152ebf6c5cab2ef008397b5a71ac13255a735",
"size": 666,
[...]
}
$
```
Tree-SHA512: 279be3818141edd3cc194a9ee65929331920afb30297ab2d6da07293a2d7311afee5c8b00c6457477d9f1f86e86786a9b56878ea3ee19fa2629b829d042d0cda
Backport of Core PR 10275
https://github.com/bitcoin/bitcoin/pull/10275/files
Completes T562
Test Plan:
make check
test_runner.py
Reviewers: deadalnix, Fabien, #bitcoin_abc
Reviewed By: Fabien, #bitcoin_abc
Subscribers: schancel
Differential Revision: https://reviews.bitcoinabc.org/D2746
* Backport remaining changes from Core PR 10742
Summary:
Partial backport of Core PR 10742
https://github.com/bitcoin/bitcoin/pull/10742/files
Completes T549
Test Plan:
make check
test_runner.py
Reviewers: #bitcoin_abc, deadalnix, Fabien
Reviewed By: #bitcoin_abc, Fabien
Subscribers: teamcity, schancel
Differential Revision: https://reviews.bitcoinabc.org/D2719
* Use static_cast instead of C-style casts for non-fundamental types
Summary:
A C-style cast is equivalent to try casting in the following order:
1. const_cast(...)
2. static_cast(...)
3. const_cast(static_cast(...))
4. reinterpret_cast(...)
5. const_cast(reinterpret_cast(...))
By using static_cast<T>(...) explicitly we avoid the possibility
of an unintentional and dangerous reinterpret_cast. Furthermore
static_cast<T>(...) allows for easier grepping of casts. Please enter the commit message for your changes. Lines starting
Backport of Core PR10498
bitcoin/bitcoin#10498
Completes T555
Test Plan:
make check
test_runner.py
Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc
Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc
Subscribers: schancel
Differential Revision: https://reviews.bitcoinabc.org/D2765
* Fix Windows build errors introduced in D2765
Summary:
See core issue #12386:
bitcoin/bitcoin#12386
Backport of core PR12416
Test Plan:
make check
Build the windows binaries
Reviewers: #bitcoin_abc, deadalnix, jasonbcox
Reviewed By: #bitcoin_abc, jasonbcox
Subscribers: schancel
Differential Revision: https://reviews.bitcoinabc.org/D2779
* [Target v0.19] Deprecate and add test for signrawtransaction
Summary:
Deprecate signrawtransaction (target is v0.19).
Backport 3/3 of core PR10579 (part of commit d602348 + deprecation from 1e79c05)
Depends on D2763
Completes task T438
Test Plan:
make check
./test/functional/test_runner.py --extended
Reviewers: #bitcoin_abc, jasonbcox, deadalnix
Reviewed By: #bitcoin_abc, jasonbcox, deadalnix
Subscribers: deadalnix, teamcity
Differential Revision: https://reviews.bitcoinabc.org/D2009
* Add schnorr sign benchmark
Summary:
As per title.
Depends on D2747.
Test Plan: ./bench_sign
Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien, jasonbcox
Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien, jasonbcox
Subscribers: jasonbcox, Fabien, schancel
Differential Revision: https://reviews.bitcoinabc.org/D2748
* Add schnorr verify benchmark
Summary:
As per title.
Depends on D2747.
Test Plan: ./bench_verify
Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien, jasonbcox
Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien, jasonbcox
Subscribers: jasonbcox, Fabien, schancel
Differential Revision: https://reviews.bitcoinabc.org/D2749
* Update wallet_listsinceblock.py to use signrawtransactionwithwallet rpc
Summary:
This has been added before D2009 gets landed, it needs to be updated as
`signrawtransaction` is going to get deprecated in favor of
`signrawtransactionwithwallet` and `signrawtransactionwithkey`.
Test Plan:
./test/functional/test_runner.py wallet_listsinceblock
Reviewers: #bitcoin_abc, deadalnix, jasonbcox
Reviewed By: #bitcoin_abc, jasonbcox
Subscribers: schancel
Differential Revision: https://reviews.bitcoinabc.org/D2763
* Document method for reviewers to verify chainTxData
Summary:
This commit adds the final block hash of the window to getchaintxstats
and documents how reviewers can verify changes to chainTxData.
Backport of PR12317: bitcoin/bitcoin#12317
Completes T566.
Test Plan:
make check
test_runner.py rpc_blockchain
Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc
Reviewed By: jasonbcox, Fabien, O1 Bitcoin ABC, #bitcoin_abc
Subscribers: teamcity, schancel
Differential Revision: https://reviews.bitcoinabc.org/D2711
* qa: Improve getchaintxstats functional test
Summary:
Completes T561.
Depends on D2711.
Backport of PR12083
https://github.com/bitcoin/bitcoin/pull/12083/files
Test Plan:
make check
test_runner.py
Reviewers: Fabien, deadalnix, O1 Bitcoin ABC, #bitcoin_abc, nakihito
Reviewed By: Fabien, O1 Bitcoin ABC, #bitcoin_abc
Subscribers: teamcity, schancel
Differential Revision: https://reviews.bitcoinabc.org/D2702
…ions when parameter is a reorg'd block 876e92b Testing: listsinceblock should display all transactions that were affected since the given block, including transactions that were removed due to a reorg. (Karl-Johan Alm) f999c46 listsinceblock: optionally find and list any transactions that were undone due to reorg when requesting a non-main chain block in a new 'removed' array. (Karl-Johan Alm) Pull request description: The following scenario will not notify the caller of the fact `tx0` has been dropped: 1. User 1 receives BTC in tx0 from utxo1 in block aa1. 2. User 2 receives BTC in tx1 from utxo1 (same) in block bb1 3. User 1 sees 2 confirmations at block aa3. 4. Reorg into bb chain. 5. User 1 asks `listsinceblock aa3` and does not see that tx0 is now invalidated. See `listsinceblock.py` commit for related test. The proposed fix is to iterate from the given block down to the fork point, and to check each transaction in the blocks against the wallet, in addition to including all transactions from the fork point to the active chain tip (the current behavior). Any transactions that were present will now also be listed in the `listsinceblock` output in a new `replaced` array. This operation may be a bit heavy but the circumstances (and perceived frequency of occurrence) warrant it, I believe. Example output: ```Python { 'transactions': [], 'replaced': [ { 'walletconflicts': [], 'vout': 1, 'account': '', 'timereceived': 1485234857, 'time': 1485234857, 'amount': '1.00000000', 'bip125-replaceable': 'unknown', 'trusted': False, 'category': 'receive', 'txid': 'ce673859a30dee1d2ebdb3c05f2eea7b1da54baf68f93bb8bfe37c5f09ed22ff', 'address': 'miqEt4kWp9zSizwGGuUWLAmxEcTW9bFUnQ', 'label': '', 'confirmations': -7 } ], 'lastblock': '7a388f27d09e3699102a4ebf81597d974fc4c72093eeaa02adffbbf7527f6715' } ``` I believe this addresses the comment by @luke-jr in bitcoin#9516 (comment) but I could be wrong.. Tree-SHA512: 607b5dcaeccb9dc0d963d3de138c40490f3e923050b29821e6bd513d26beb587bddc748fbb194503fe618cfe34a6ed65d95e8d9c5764a882b6c5f976520cff35
…en parameter is a reorg'd block
876e92b Testing: listsinceblock should display all transactions that were affected since the given block, including transactions that were removed due to a reorg. (Karl-Johan Alm)
f999c46 listsinceblock: optionally find and list any transactions that were undone due to reorg when requesting a non-main chain block in a new 'removed' array. (Karl-Johan Alm)
Pull request description:
The following scenario will not notify the caller of the fact `tx0` has been dropped:
1. User 1 receives BTC in tx0 from utxo1 in block aa1.
2. User 2 receives BTC in tx1 from utxo1 (same) in block bb1
3. User 1 sees 2 confirmations at block aa3.
4. Reorg into bb chain.
5. User 1 asks `listsinceblock aa3` and does not see that tx0 is now invalidated.
See `listsinceblock.py` commit for related test.
The proposed fix is to iterate from the given block down to the fork point, and to check each transaction in the blocks against the wallet, in addition to including all transactions from the fork point to the active chain tip (the current behavior). Any transactions that were present will now also be listed in the `listsinceblock` output in a new `replaced` array. This operation may be a bit heavy but the circumstances (and perceived frequency of occurrence) warrant it, I believe.
Example output:
```Python
{
'transactions': [],
'replaced': [
{
'walletconflicts': [],
'vout': 1,
'account': '',
'timereceived': 1485234857,
'time': 1485234857,
'amount': '1.00000000',
'bip125-replaceable': 'unknown',
'trusted': False,
'category': 'receive',
'txid': 'ce673859a30dee1d2ebdb3c05f2eea7b1da54baf68f93bb8bfe37c5f09ed22ff',
'address': 'miqEt4kWp9zSizwGGuUWLAmxEcTW9bFUnQ',
'label': '',
'confirmations': -7
}
],
'lastblock': '7a388f27d09e3699102a4ebf81597d974fc4c72093eeaa02adffbbf7527f6715'
}
```
I believe this addresses the comment by @luke-jr in bitcoin/bitcoin#9516 (comment) but I could be wrong..
Tree-SHA512: 607b5dcaeccb9dc0d963d3de138c40490f3e923050b29821e6bd513d26beb587bddc748fbb194503fe618cfe34a6ed65d95e8d9c5764a882b6c5f976520cff35
When
listsinceblockis called with a block hash that belongs to a non-main chain, the system will mistakenly use its relative position in its chain vs the current active chain's height to determine which transactions should be included.This patch locates the fork point and uses that as reference instead.
The problem can be observed in the following scenario: with address
Abelonging to noden1in a network withn1throughn4:n12andn34.txfromn3to addressA.n1. Note last block hash as<lastKnownHash>.n3.n1234.n3to activaten34chain.listsinceblock <lastKnownHash>onn1.The resulting transaction list should contain
tx, but it does not.