71d0344cf25d3aaf60112c5248198c444bc98105 docs: release note wording (Karl-Johan Alm)
3d2ff379131a01e4e9f9648b150e806104a23795 wallet/rpc: use static help text (Karl-Johan Alm)
53c3c1ea9e20f881c843a9219e48cec202e962f8 wallet/rpc/getbalances: add entry for 'mine.used' balance in results (Karl-Johan Alm)
Pull request description:
This addresses a few remaining issues pointed out in #13756:
* First commit addresses https://github.com/bitcoin/bitcoin/pull/13756#discussion_r284907468
* Second commit addresses https://github.com/bitcoin/bitcoin/pull/13756#discussion_r294868973
Ping jnewbery and achow101 as they pointed out these issues.
ACKs for commit 71d034:
jnewbery:
ACK 71d0344cf25d3aaf60112c5248198c444bc98105
meshcollider:
re-utACK 71d0344cf2
Tree-SHA512: 5e28822af0574ad07dbbed21aa2fe7866bf5770b4c0a1c150ad0da8af3152bcfb7170330a7497fa500326c594740ecf63733cf58325821e2811d7b911d5783a0
facfb4111d14a3b06c46690a2cca7ca91cea8a96 rpc: Deprecate getunconfirmedbalance and getwalletinfo balances (MarcoFalke)
999931cf8f167c7547f1015cdf05437a460c27f0 rpc: Add getbalances RPC (MarcoFalke)
fad13e925e197163a942f3f0d1ba2c95a2b65a56 rpcwallet: Make helper methods const on CWallet (MarcoFalke)
fad40ec9151248c6e8225e14980424f581d23e02 wallet: Use IsValidNumArgs in getwalletinfo rpc (MarcoFalke)
Pull request description:
This exposes the `CWallet::GetBalance()` struct over RPC.
In the future, incorrectly named rpcs such as `getunconfirmedbalance` or rpcs redundant to this such as `getbalance` could be removed.
ACKs for commit facfb4:
jnewbery:
utACK facfb4111d14a3b06c46690a2cca7ca91cea8a96
Tree-SHA512: 1f54fedce55df9a8ea82d2b6265354b39a956072621876ebaee2355aac0e23c7b64340c3279502415598c095858529e18b50789be956250aafda1cd3a8d948a5
6fc554f591d8ea1681b8bb25aa12da8d4f023f66 wallet: Reset reused transactions cache (Fabian Jahr)
Pull request description:
Fixes#17603 (together with #17824)
`getbalances` is using the cache within `GetAvailableCredit` under certain conditions [here](35fff5be60/src/wallet/wallet.cpp (L1826)). For a wallet with `avoid_reuse` activated this can lead to inconsistent reporting of `used` transactions/balances between `getbalances` and `listunspent` as pointed out in #17603. When an address is reused before the first transaction is spending from this address, the cache is not updated even after the transaction is sent. This means the remaining outputs at the reused address are not showing up as `used` in `getbalances`.
With this change, any newly incoming transaction belonging to the wallet marks all the other outputs at the same address as dirty.
ACKs for top commit:
kallewoof:
Code review re-ACK 6fc554f591d8ea1681b8bb25aa12da8d4f023f66
promag:
ACK 6fc554f591d8ea1681b8bb25aa12da8d4f023f66.
achow101:
Re-ACK 6fc554f591d8ea1681b8bb25aa12da8d4f023f66
meshcollider:
Code review ACK 6fc554f591d8ea1681b8bb25aa12da8d4f023f66
Tree-SHA512: c4cad2c752176d16d77b4a4202291d20baddf9f27250896a40274d74a6945e0f6b34be04c2f9b1b2e756d3ac669b794969df8f82a98e0b16f10e92f276649ea2
8f2d7737cc236b6122f30e31856eb3181960fba1 test: add functional test for non-standard txs with too large scriptSig (Sebastian Falbesoner)
Pull request description:
Approaches another missing functional test of issue #17394 (counterpart to unit test in PR #17480, Commit 5e8a56348b): A transaction is rejected by the mempool with reason `"scriptsig-size"` if any of the inputs' scriptSig is larger than 1650 bytes.
ACKs for top commit:
MarcoFalke:
ACK 8f2d7737cc236b6122f30e31856eb3181960fba1
instagibbs:
ACK 8f2d7737cc
Tree-SHA512: 7a45b8a4181158be3e3b91756783ddf032f132ca8780dc35fac91b2df2149268f784d28ac56005135c4d86a357c57805c5a54b8155f0d049932844b18dc03992
1be0b1fb2adcf95d76f879195564c0bf84162e31 test: add functional test for non-standard bare multisig txs (Sebastian Falbesoner)
Pull request description:
Approaches another missing functional test of issue #17394 (counterpart to unit test in PR #17502): A transaction is rejected by the mempool with reason `"bare-multisig"` if any of the outputs' scriptPubKey has bare multisig format (`M <PubKey1> <PubKey2> ... <PubKeyN> N OP_CHECKSIG`) and bitcoind is started with the argument `-permitbaremultisig=0`.
ACKs for top commit:
instagibbs:
utACK 1be0b1fb2a
kristapsk:
ACK 1be0b1fb2adcf95d76f879195564c0bf84162e31
Tree-SHA512: 2cade68c4454029b62278b38d0f137c2605a0e4450c435cdda2833667234edd4406f017ed12fa8df9730618654acbaeb68b16dcabb9f5aa84bad9f1c76c6d476
fa40e48c50d8ccf42ce5e66c12390e2ed4b60e75 ci: Remove unparseable lines from supp file for old xenial clang tsan (MarcoFalke)
fa1bfc476c9208a4c412c8ca74d05f52bb47766f ci: ubsan report_error_type=1 and add suppressions (MarcoFalke)
fa69cef13e5aab8264339eb3d50a9e89d59efd87 test: Print stderr when subprocess fails (MarcoFalke)
2222c305866a77065ab5be24c1c252bae252bb59 test: Use char instead of unsigned char (MarcoFalke)
faa8023ce9a47b282e1fac3ca8b3a7bb0042935a ci: Bump to clang-8 for asan build to avoid segfaults on ppc64le (MarcoFalke)
Pull request description:
Use clang-8 instead of default clang (which is clang-6 on Bionic) to avoid spurious segfaults when running the ci system on ppc64le
ACKs for top commit:
practicalswift:
ACK fa40e48c50d8ccf42ce5e66c12390e2ed4b60e75 assuming Travis is happy -- diff looks correct :)
Tree-SHA512: f4f26232d3a0ef38da245869340f723d279a3db9823befbc735fb5a00024dae041c7306d7ae55d2488e6f86aa96cdea155b007aefb561fba505141e8dbc717dc
900d8f6f70859f528e84c5c38d0332f81d19df55 util: Disallow network-qualified command line options (Russell Yanofsky)
Pull request description:
Previously these were allowed but ignored.
This change implements one of the settings simplifications listed in #17508. Change includes release notes.
ACKs for top commit:
laanwj:
ACK 900d8f6f70859f528e84c5c38d0332f81d19df55
Tree-SHA512: ab020a16a86c1e8ec709fbf798d533879d32c565eceeb7eb785c33042c49c6b4d1108c5453d8166e4a2abffc2c8802fbb6d3b895e0ddeefa8f274fd647e3c8ad
ff59bcd3213ef61f2167c0aa60fcaf5afbc20c61 gui: Drop PeerTableModel dependency to ClientModel (João Barbosa)
Pull request description:
Class `PeerTableModel` doesn't actually depend on `ClientModel`.
ACKs for top commit:
Empact:
Code Review ACK ff59bcd321
hebasto:
ACK ff59bcd3213ef61f2167c0aa60fcaf5afbc20c61, tested on Linux Mint 19.3. No changes in behavior are observed.
Tree-SHA512: 29fa3c316c05b8f7b9340e5859bbb8c3a0b826aa7c865c892cfa13b5ad30f822fcaae4e01555f7860cd1727f20b7ef555a808235522a04a6eebaaa7b605f8595
cb8a86d9f952401eaad68b2e3818ce50f7befd91 gui: Remove WalletView and BitcoinGUI circular dependency (João Barbosa)
ac3d10777d65b68862c6deb57594c8fc4d21ca77 gui: Add transactionClicked and coinsSent signals to WalletView (João Barbosa)
Pull request description:
Essentially moves the code in `WalletView::setBitcoinGUI` to the only caller. Two new signals are added beforehand in the first commit so that the connections in `WalletFrame` are all from the wallet view.
ACKs for top commit:
hebasto:
ACK cb8a86d9f952401eaad68b2e3818ce50f7befd91, tested on Linux Mint 19.3.
jonasschnelli:
utACK cb8a86d9f952401eaad68b2e3818ce50f7befd91
Tree-SHA512: 250316cd3689e51c8cded9ccd75963c836dcafa6db25d684f2aa691dea9738895f9140793e0f925784909e39f8257f7e1c7d611e8bd6d6634e1a50333f4ddb1e
3aee10b80b9d9a0f5172fc2ee75f03a37d5c3863 gui: Drop ShutdownWindow dependency to BitcoinGUI (João Barbosa)
61eb058cc10592cfa314ba2209fb370706100e8b gui: Drop BanTableModel dependency to ClientModel (João Barbosa)
Pull request description:
`ShutdownWindow::showShutdownWindow` just needs a widget to center the shutdown window and to borrow its title.
ACKs for top commit:
hebasto:
ACK 3aee10b80b9d9a0f5172fc2ee75f03a37d5c3863, since previous review only suggested change `QWidget` --> `QMainWindow`
jonasschnelli:
utACK 3aee10b80b9d9a0f5172fc2ee75f03a37d5c3863
Tree-SHA512: e15cb6ee274730bd071d3d97b540c5059e5c655248d69a37c3fd00f2aacc6cfcb36b9a65755718027e15482ec8e5e85534c1dc13d0ddb4e0680df03fbf6571f2
* tests: extend "age" period in `feature_llmq_connections.py`
see `NOTE`
* tests: sleep more in `wait_until` by default
Avoid overloading rpc with 20+ requests per second, 2 should be enough.
* tests: various fixes in `activate_dip0024`
- lower batch size
- no fast mode
- disable spork17 while mining
- bump mocktime on every generate call
* tests: bump mocktime on generate in `activate_dip8`
* tests: fix `reindex` option in `restart_mn`
Make sure nodes actually finished reindexing before moving any further.
* tests: trigger recovery threads and wait on mn restarts
* tests: sync blocks in `wait_for_quorum_data`
* tests: bump disconnect timeouts in `p2p_invalid_messages.py`
1 is too low for busy nodes
* tests: Wait for addrv2 processing before bumping mocktime in p2p_addrv2_relay.py
* tests: use `timeout_scale` option in `get_recovered_sig` and `isolate_node`
* tests: fix `wait_for...`s
* tests: fix `close_mn_port` banning test
* Bump MASTERNODE_SYNC_RESET_SECONDS to 900
This helps to avoid issues with 10m+ bump_mocktime on isolated nodes in feature_llmq_is_retroactive.py and feature_llmq_simplepose.py.
* style: fix extra whitespace
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
fac2fc4dd8a28b99e17c57e4ab6580a3231f1d0a test: Increase debugging to hunt down mempool_reorg intermittent failure (MarcoFalke)
Pull request description:
Top commit has no ACKs.
Tree-SHA512: 4094b44afaa623e58b69f8d0332e60f0150b9ae2fd8bb265210d85546d887672ab8a3435cd9b086be14f69ab5b17e0f9fae06bd8aec1e7947ca766dd72b577c4
fae98668d150bae7a68167749ae134894cb1140e test: Fix intermittent error in mempool_reorg (MarcoFalke)
Pull request description:
Example: https://travis-ci.org/github/bitcoin/bitcoin/jobs/677689899#L4717
Also speed up tx relay and fix two pep8 errors while touching the file anyway.
ACKs for top commit:
vasild:
utACK fae9866
Tree-SHA512: 23a7894e71ad0e1a59c74c73643708fca21b505fa4e980038d554294063fd63c396669eefb233ffdffb0083968e51b702c643cb449df8f656dd8345a20f33907
fa192952507dbe5f405abffce38b3556923ba271 test: Bump timeouts to avoid valgrind failures (MarcoFalke)
Pull request description:
This should fix ci timeout issues such as:
* https://travis-ci.org/github/bitcoin/bitcoin/jobs/661946972#L3109
* https://travis-ci.org/github/bitcoin/bitcoin/jobs/662521570#L4922
ACKs for top commit:
practicalswift:
ACK fa192952507dbe5f405abffce38b3556923ba271
Tree-SHA512: 150f804957575033a83fd47c68fd6adff2c11b110ec5803bd77f121256349805b595c39a3a5047738ec538d082ee38cebbcb6792c787ef22f56b8dd0d9618c1f
2d23082cbe4641175d752a5969f67cdadf1afcea bump test timeouts so that functional tests run in valgrind (Micky Yun Chan)
Pull request description:
ci/tests: Bump timeouts so all functional tests run on travis in valgrind #17763
Top commit has no ACKs.
Tree-SHA512: 5a8c6e2ea02b715facfcb58c761577be15ae58c45a61654beb98c2c2653361196c2eec521bcae4a9a1bab8e409d6807de771ef4c46d3d05996ae47a22d499d54
faec689bed7a5b66e2a7675853d10205b933cec8 txmempool: Make entry time type-safe (std::chrono) (MarcoFalke)
faaa1f01daba94b021ca77515266a16d27f0364e util: Add count_seconds time helper (MarcoFalke)
1111170f2f0141084b5b4ed565b2f07eba48599a test: mempool entry time is persisted (MarcoFalke)
Pull request description:
This changes the type of the entry time of txs into the mempool from `int64_t` to `std::chrono::seconds`.
The benefits:
* Documents the type for developers
* Type violations result in compile errors
* After compilation, the two are equivalent (at no run time cost)
ACKs for top commit:
ajtowns:
utACK faec689bed7a5b66e2a7675853d10205b933cec8
laanwj:
ACK faec689bed7a5b66e2a7675853d10205b933cec8
Tree-SHA512: d958e058755d1a1d54cef536a8b30a11cc502b7df0d6ecf84a0ab1d38bc8105a67668a99cd5087a444f6de2421238111c5fca133cdf8e2e2273cb12cb6957845
faf45d1f1f997c316fc4c611a23c4456533eefe9 http: Avoid crash when g_thread_http was never started (MarcoFalke)
fa12a37b27f0570a551b8c103ea6537ee4a8e399 test: Replace inline-comments with logs, pep8 formatting (MarcoFalke)
fa83b39ff3ae3fbad93df002915c0e5f99c104a9 init: Remove confusing and redundant InitError (MarcoFalke)
Pull request description:
Avoid a crash during shutdown when the init sequence failed for some reason
ACKs for top commit:
promag:
Tested ACK faf45d1f1f997c316fc4c611a23c4456533eefe9.
ryanofsky:
Code review ACK faf45d1f1f997c316fc4c611a23c4456533eefe9. Thanks for updates, this is much easier to parse for me now. Since previous reviews: split out and reverted some cleanups & replaced chmod with mkdir in test
hebasto:
ACK faf45d1f1f997c316fc4c611a23c4456533eefe9, tested on Linux Mint 19.3 with the following patch:
Tree-SHA512: 59632bf01c999e65c724e2728ac103250ccd8b0b16fac19d3a2a82639ab73e4f2efb86c78e63c588a5954625d8d0cf9545e2a7e070e6e15d2a54beeb50e00b61
66fe7b1a98c03f690dcf60d359baac124658aeae test: added test for upgradewallet RPC (Harris)
Pull request description:
This PR adds tests for the newly merged *upgradewallet* RPC.
Additionally, it expands `test_framework/util.py` by adding the function `adjust_bitcoin_conf_for_pre_17` to support nodes that don't parse configuration sections.
This test uses two older node versions, v0.15.2 and v0.16.3, to create older wallet versions to be used by `upgradewallet`.
Fixes https://github.com/bitcoin/bitcoin/issues/18767
Top commit has no ACKs.
Tree-SHA512: bb72ff1e829e2c3954386cc308842820ef0828a4fbb754202b225a8748f92d4dcc5ec77fb146bfd5484a5c2f29ce95adf9f3fb4483437088ff3ea4a8d2c442c1
fa03713e133e3017112fdd5c278e0c8643054578 test: Properly raise FailedToStartError when rpc shutdown before warmup finished (take 2) (MarcoFalke)
Pull request description:
actually (?) fix#18561
See most recent traceback https://travis-ci.org/github/bitcoin/bitcoin/jobs/674668692#L7062
I believe the reason the error is still there is that ConnectionResetError is derived from OSError:
ConnectionResetError(ConnectionError(OSError))
And IOError is an alias for OSError since python 3.3, see https://docs.python.org/3/library/exceptions.html#IOError
So fix that by renaming IOError to the alias OSError and move the less specific catch clause down a few lines.
ACKs for top commit:
jonatack:
ACK fa03713e133e3017112fdd5c278e0c8643054578
Tree-SHA512: 6e5b214ed9101bf8ebe7472dcc1f9e9d128e2575c93ec00c8d0774ae1a9b52a8c2a653a45a0eab8d881570b08dd5ffeddf5aca88a10438c366e1f633253cb0b5
38677274f931088218eeb1f258077d3387f39c89 rpc: settxfee respects -maxtxfee wallet setting (Fabian Jahr)
bda84a08a0ac92dff6cadc99cf9bb8c3fadd7e13 rpc: Add documentation for deactivating settxfee (Fabian Jahr)
Pull request description:
~~Closes 18315~~
`settxfee` can be deactivated by passing 0 as the fee. That does not seem to be documented for the user so this PR adds it in the description. The return value of a simple boolean seems also too simplified given the multiple dimensions that this deactivation feature enables. I.e. it does not seem intuitive if the returned boolean shows that the call succeeded or if means that `settxfee` is active. My suggested solution is to change the return value to a JSON object that included the "active" state and the currently set fee rate.
Examples:
```
$ src/bitcoin-cli settxfee 0.0000000
{
"active": false,
"fee_rate": "0.00000000 BTC/kB"
}
$ src/bitcoin-cli settxfee 0.0001
{
"active": true,
"fee_rate": "0.00010000 BTC/kB"
}
```
ACKs for top commit:
MarcoFalke:
ACK 38677274f931088218eeb1f258077d3387f39c89, seems useful to error out early instead of later #16257🕍
jonatack:
ACK 38677274f931088218eeb
meshcollider:
LGTM, utACK 38677274f931088218eeb1f258077d3387f39c89
Tree-SHA512: 642813b5cf6612abb4b6cb63728081a6bd1659d809e0149c8f56060b6da7253fee989b3b202854f3051df3773c966799af30b612648c466b099f00590f356548
cc84460c164bcb2a874d4f08b3a2624e5ee9ff0a test: move sync_blocks and sync_mempool functions to test_framework.py (Roy Shao)
Pull request description:
This PR moves `sync_blocks` and `sync_mempool` out from `test_framework/util.py` to `test_framework/test_framework.py` so they can take contextual information of test framework into account.
* Change all reference callers to call functions from `test_framework.py`
* Remove `**kwargs` which is not used
* Take into account of `timeout_factor` when respecting timeout in function implementations.
* Pass all tests by running `./test/functional/test_runner.py`
fixes#18930
ACKs for top commit:
MarcoFalke:
ACK cc84460c164bcb2a874d4f08b3a2624e5ee9ff0a , reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space 💫
Tree-SHA512: a79b2a3fa842fc26a7aacb834bb2aea88b3049916c0b754e60002a77ce94bb5954e0ea3b436bf268e9295efb62d721dfef263a09339a55c684ac3fda388c275e
fac3716b09bb9ee121db629873d9694a95cae942 test: check that peer is connected when calling sync_* (MarcoFalke)
Pull request description:
Without a connection there is no way to sync, so we can fail early and don't have to wait for the timeout
ACKs for top commit:
jonatack:
ACK fac3716b09bb9
Tree-SHA512: 12f771473c23e152dae4bfb201fadb2c3530cf439de64fea07d048734614543080a5d05c9c36e6e398c6a69c8279f609d34706599571814172a11bcfbea4a3b9
