fa49db7eac doc: Clarify sendrawtransaction::maxfeerate==0 help (MarcoFalke)
Pull request description:
Used a lot in e.g. the tests: `git grep 'maxfeerate=0)' test`
ACKs for commit fa49db:
promag:
ACK fa49db7.
jonatack:
ACK fa49db7eac
Tree-SHA512: cb3fa10960f45606c3599b76c48666a663e5c44cfb7c29bab5d44caa7dc6cb57aaac81cb9b173e079dde01d07c5363c99416f25303a8fd41010928118474a741
faf62d9415 gui: Generate bech32 addresses by default (MarcoFalke)
Pull request description:
Most services support bech32 addresses now, so generating legacy addresses by default seems overly cautious. bech32 addresses are more robust and user friendly in multiple ways.
ACKs for commit faf62d:
promag:
utACK faf62d9, maybe add a release note "checkbox changed, but the behavior/outcome remains the same".
laanwj:
utACK faf62d9415, don't think a release note is needed for this specifically, though a general release note on switching to bech32 by default would make sense
fanquake:
tACK faf62d9
Empact:
utACK faf62d9415
Tree-SHA512: a03e6ccf1e5476fe800941992c531664830cec0644984d8bce97c8d8f5d6d9abd528ab979892368cd195e7c8b9083d6b8dfd36a7cb172aa9dbeaa01a948e30e1
6d44c5ebf9 depends: Add commands for each package for each stage (Carl Dong)
80f0e05b70 depends: Preprocessing doesn't care about deps (Carl Dong)
Pull request description:
Adds make targets for each package for each stage, e.g.
```sh
make zeromq_configured
```
ACKs for commit 6d44c5:
MarcoFalke:
ACK 6d44c5ebf9 (Haven't looked at the code changes, but adding this feature makes sense)
ryanofsky:
ACK 6d44c5ebf9
Tree-SHA512: f1ac0aecfd2372aed09ca63603e2634552cb3f6ff9d610f958e2a66952d7d9e870b4c32b7d996886879e6d3016532272e8b1a10c13ed7b31009c6c96f786db9f
8f5d9431a Add regtests for HTTP status codes. (Daniel Kraft)
Pull request description:
This adds explicit tests for the returned HTTP status codes to `interface_rpc.py` (for error cases) and the HTTP JSON-RPC client in general for success.
#15381 brought up discussion about the HTTP status codes in general, and the general opinion was that the current choice may not be ideal but should not be changed to preserve compatibility with existing JSON-RPC clients. Thus it makes sense to actually test the current status to ensure this desired compatibility is not broken accidentally.
ACKs for commit 8f5d94:
laanwj:
utACK 8f5d9431a3
promag:
utACK 8f5d943.
jonasschnelli:
utACK 8f5d9431a3
Tree-SHA512: 82503ccd134dd9145304e95cb6c61755f100bee27593d567cdd5c0c554d47e7b06d937456cab04107f46f4984930355db65d5e711008a0b05f2b8feec9f2950e
7813eb1db1 [qa] Overhaul p2p_compactblocks.py (Suhas Daftuar)
Pull request description:
Remove tests of:
- compactblock behavior in a simulated pre-segwit version of bitcoind
This should have been removed a long time ago, as it is not generally
necessary for us to test the behavior of old nodes (except perhaps if we
want to test that upgrading from an old node to a new one behaves properly)
- compactblock behavior during segwit upgrade (ie verifying that network
behavior before and after activation was as expected)
This is unnecessary to test now that segwit activation has already happened.
ACKs for commit 7813eb:
jnewbery:
utACK 7813eb1db1
Tree-SHA512: cadf035e6f822fa8cff974ed0c2e88a1d4d7da559b341e574e785fd3d309cc2c98c63bc05479265dc00550ae7b77fc3cbe815caae7f68bcff13a04367dca9b52
892eff05f1 Add documentation of struct PSBTAnalysis et al (Glenn Willen)
ef22fe8c1f Refactor analyzepsbt for use outside RPC code (Glenn Willen)
afd20a25f2 Move PSBT decoding functions from core_io to psbt.cpp (Glenn Willen)
Pull request description:
Refactor the analyzepsbt RPC into (1) an AnalyzePSBT function, which returns
its output as a new strongly-typed PSBTAnalysis struct, and (2) a thin wrapper
which converts the struct into a UniValue for RPC use.
----
As with my previous refactoring PR, I need this because I am creating a dependency on this code from the GUI. Per discussion in #bitcoin-core-dev on IRC, since we don't want to create a dependency on UniValue in anything outside RPC, I introduced some new structs to hold the info we get when analyzing a PSBT. For the field types, I used whatever types are already used internally for this data (e.g. CAmount, CFeeRate, CKeyID), and only convert to int/string etc. in the wrapper.
@achow101, maybe take the first look? :-)
ACKs for commit 892eff:
sipa:
utACK 892eff05f1
achow101:
utACK 892eff05f1
ryanofsky:
utACK 892eff05f1. Just small cleanups since the last review: removing unneeded include, forward decl, adding const ref
Tree-SHA512: eb278b0a82717ebc3eb0c08dc5bb4eefb996a317a6a3a8ecf51cd88110ddbb188ad3482cdd9563e557995e73aca5a282c1f6e352bc598155f1203b7b46fe5dee
fa8548c5d1 net: Remove unused unsanitized user agent string CNode::strSubVer (MarcoFalke)
Pull request description:
I fail to see a use case for this unsanitized byte array. In fact this can easily be confused with `cleanSubVer` and be displayed to the user (or logged) by a simple typo that is hard to find in review.
Further reading: https://btcinformation.org/en/developer-reference#version
ACKs for commit fa8548:
promag:
utACK fa8548c, good catch.
practicalswift:
utACK fa8548c5d1
sipa:
utACK fa8548c5d1
Tree-SHA512: 3c3ff1504d1583ad099df9a6aa761458a82ec48a58ef7aaa9b5679a5281dd1b59036ba2932ed708488951a565b669a3083ef70be5a58472ff8677b971162ae2f
fabfb79673 doc: Add release notes for 15596 (MarcoFalke)
fac1a0fe54 wallet: Remove unused GetLegacyBalance (MarcoFalke)
faa3a246e8 scripted-diff: wallet: Rename pcoin to wtx (MarcoFalke)
fae5f874d5 rpc: Document that minconf is an ignored dummy value (MarcoFalke)
Pull request description:
Other RPCs such as `sendtoaddress` don't have this option at all and `sendmany` should by default spend from (lets say) our change.
ACKs for commit fabfb7:
jnewbery:
utACK fabfb79673
ryanofsky:
utACK fabfb79673. Nice writeup! Release notes are only change since previous review.
Tree-SHA512: 2526ead2330be7c2beb78b96bc5e55440566c4a3a809bbbd66f5c9fc517f6890affa5d14005dc102644d49679a374510f9507255e870cf88aaa63e429beef658
ac67582ff7 depends: latest rapidcheck, use INSTALL_ALL_EXTRAS (fanquake)
Pull request description:
This updates RapidCheck to the latest version available from https://github.com/emil-e/rapidcheck.
RapidCheck now uses the new `RC_INSTALL_ALL_EXTRAS` option, to install the extra `boost_test` packages, which should unblock progress in #14430.
ACKs for commit ac6758:
MarcoFalke:
utACK ac67582ff7
Tree-SHA512: a4a4ef0ec09cf61cdc0de241703f5f8e98f6fa92f4024a0fbbf4d4ef91d9d3bc8d662c55d896aced8de68aa9429728b2bc5001c91c6f92d63d60c47f5adf41a0
ea1a2d8794 [wallet] Remove ResendWalletTransactionsBefore (John Newbery)
f5162458cd [rpc] remove resendwallettransactions RPC (John Newbery)
Pull request description:
Remove resendwallettransactions RPC method
This RPC was added for testing wallet rebroadcasts. Since we now have a real test for wallet rebroadcasts, it's no longer needed.
The call in wallet_basic.py can be removed because wallet_resendwallettransactions.py tests wallet rebroadcast.
ACKs for commit ea1a2d:
MarcoFalke:
re-utACK ea1a2d8794
promag:
utACK ea1a2d8.
Tree-SHA512: 48245d947be1a2d2b8c30d2946105818c454a03b70b63534ecadf2144da64dafe1c9527ea670a5f4d1acd05ccdfc6c9be43ca636ee2ba58a8b7a7b2fc7bc88fd
Remove tests of:
- compactblock behavior in a simulated pre-segwit version of bitcoind
This should have been removed a long time ago, as it is not generally
necessary for us to test the behavior of old nodes (except perhaps if we
want to test that upgrading from an old node to a new one behaves properly)
- compactblock behavior during segwit upgrade (ie verifying that network
behavior before and after activation was as expected)
This is unnecessary to test now that segwit activation has already happened.
Includes changes by John Newbery.
866c8058a7 Interrupt orphan processing after every transaction (Pieter Wuille)
6e051f3d32 [MOVEONLY] Move processing of orphan queue to ProcessOrphanTx (Pieter Wuille)
9453018fdc Simplify orphan processing in preparation for interruptibility (Pieter Wuille)
Pull request description:
As individual orphan transactions can be relatively expensive to handle, it's undesirable to process all of them (max 100) as soon as the parent becomes available, as it pegs the net processing the whole time.
Change this by interrupting orphan handling after every transactions, and continue in the next processing slot of the peer that gave us the parent - similar to how getdata processing works now. Messages from other peers arriving in the mean time are processed normally, but other messages from the peer that gave us the parent have to wait until all orphan processing is done.
ACKs for commit 866c80:
sdaftuar:
ACK 866c8058a7
MarcoFalke:
utACK 866c8058a7
promag:
utACK 866c805. Verified refactor in 9453018fdc and moved code in 6e051f3d32. Not so sure about change in 866c8058a7 just because I'm not familiar with net processing.
Tree-SHA512: d8e8a1ee5f2999446cdeb8fc9756ed9c24f3d5cd769a7774ec4c317fc8d463fdfceec88de97266f389b715a5dfcc2b0a3abaa573955ea451786cc43b870e8cde
afc06fc868 rpc: Fix help text for signtransactionwithXXX (Torkel Rogstad)
Pull request description:
This PR fixes the help text for the `signrawtransactionwithwallet` and `signrawtransactionwithkey` RPC calls. They both marked the `amount` field in the UTXO dependencies as required. This field is omitted in the [`rpc_rawtransaction.py` test](8a8b03ecd2/test/functional/rpc_rawtransaction.py (L155)) and [`successful_signing_test`](8a8b03ecd2/test/functional/rpc_signrawtransaction.py (L42)) in `rpc_signrawtransaction.py`.
ACKs for commit afc06f:
promag:
utACK afc06fc.
MarcoFalke:
utACK afc06fc868
Tree-SHA512: 7847844ca38d8033fef2f5255700d21487d78a63ecee8b80362fa28fadfafc80ba29a89f38d7ebb3a1be4c7d47ff6c338f67afec9ee22bf065fb352bb3d03d3a
1111f0718a test: .style.yapf: Set column_limit=160 (MarcoFalke)
Pull request description:
The current style is pep8, as suggested in https://github.com/bitcoin/bitcoin/blob/master/test/functional/README.md#style-guidelines.
generated with
```
$ yapf --version
yapf 0.24.0
$ yapf --style-help --style=pep8 > .style.yapf
```
However, we don't use the column_limit of 79 right now. Practically it is somewhere between 120-240.
Some stats:
```
column_limit=120: 115 files changed, 2423 insertions(+), 1408 deletions(-)
column_limit=160: 108 files changed, 1563 insertions(+), 1247 deletions(-)
column_limit=200: 104 files changed, 1255 insertions(+), 1178 deletions(-)
ACKs for commit 1111f0:
practicalswift:
utACK 1111f0718a agree with @ryanofsky
ryanofsky:
utACK 1111f0718a
Tree-SHA512: 1ce0da83b917496f4ea7d1af31b624517a78998a10091b6ba611737f2c819fa3cda1786307f15d20131a00fd95232818e3d1108950dd12b60c4674eaa447839f
This RPC was added for testing wallet rebroadcasts. Since we now have a
real test for wallet rebroadcasts, it's no longer needed.
The call in wallet_basic.py can be removed because
wallet_resendwallettransactions.py tests wallet rebroadcast.
2a1408c3ec Comment for seemingly duplicate LIBBITCOIN_SERVER (Peter Bushnell)
Pull request description:
Added a comment to explain the addition of LIBBITCOIN_SERVER twice in bitcoind_LDADD which seems incorrect at a glance until the behaviour of Linux linkers is understood.
ACKs for commit 2a1408:
practicalswift:
ACK 2a1408c3ec
MarcoFalke:
ACK 2a1408c
fanquake:
utACK 2a1408c
ryanofsky:
utACK 2a1408c3ec
Tree-SHA512: dd2a7f61d53ce8882a56c831c32e1f48e9eab741ef21361f195c38bb455abdc4bc524d3b44b6f69c7498898cd871a23c39d215de28db3b20ef5fd2135d5e136a
03d6d23810 [tests] make pruning test faster (John Newbery)
1c29ac40fb [tests] style fixes in feature_pruning.py (John Newbery)
Pull request description:
This commit makes the pruning.py much faster.
Key insights to do this:
- pruning.py doesn't care what kind of transactions make up the big
blocks that are pruned in the test. Instead of making blocks with
several large, expensive to construct and validate transactions,
instead make the large blocks contain a single coinbase transaction with
a huge OP_RETURN txout.
- avoid stop-starting nodes where possible.
ACKs for commit 03d6d2:
MarcoFalke:
utACK 03d6d23810
Tree-SHA512: 511642ce0fa294319dce3486fe06d75970d8ab66deda7f692be081d3056b4ce5b4cf91a7b5762eefbba224ba6c848750016454ff1e5d564acc507b1c41213628
This commit makes the pruning.py much faster.
Key insights to do this:
- pruning.py doesn't care what kind of transactions make up the big
blocks that are pruned in the test. Instead of making blocks with
several large, expensive to construct and validate transactions,
instead make the large blocks contain a single coinbase transaction with
a huge OP_RETURN txout.
- avoid stop-starting nodes where possible.
This test could probably be made even faster by using the P2P interface
for submitting blocks instead of the submitblock RPC.
8b8d8eeae9 Remove travis_wait from lint script (Graham Krizek)
Pull request description:
Using the `travis_wait` command in conjunction with `set -o errexit` causes problems. The `travis_wait` command will correctly log the command's output if successful, but if the command fails the process exits before the `travis_wait` command can dump the logs. This will hide important debugging information like error messages and stack traces. We ran into this in #15196 and it was very hard to debug because output was being suppressed.
`travis_wait` was being used because the `contrib/verify-commits/verify-commits.py` script can sometimes run for a long time without producing any output. If a script runs for 10 minutes without logging anything, the CI run times out. The `travis_wait` command will extend this timeout by logging a message for you, while sending stderr and stdout to a file.
This PR removes the `travis_wait` command from our CI system and adds additional logging to the `verify-commits.py` script so it doesn't make Travis timeout.
ACKs for commit 8b8d8e:
MarcoFalke:
utACK 8b8d8eeae9
Tree-SHA512: 175a8dd3f4d4e03ab272ddba94fa8bb06875c9027c3f3f81577feda4bc8918b5f0e003a19027f04f8cf2d0b56c68633716a6ab23f95b910121a8d1132428767d
f6ee177f7 Remove unused AES-128 code (practicalswift)
Pull request description:
Remove unused AES-128 code.
As far as I can tell this AES-128 code has never been in use in the project (outside of testing/benchmarking).
The AES-256 code is used in `CCrypter::Encrypt`/`CCrypter::Decrypt` (`src/wallet/crypter.cpp`).
Trivia: 0.15% of the project's C++ LOC count (excluding dependencies) is trimmed off:
```
$ LOC_BEFORE=$(git grep -I "" HEAD~1 -- "*.cpp" "*.h" ":(exclude)src/leveldb/" ":(exclude)src/secp256k1/" ":(exclude)src/univalue/" | wc -l)
$ LOC_AFTER=$(git grep -I "" -- "*.cpp" "*.h" ":(exclude)src/leveldb/" ":(exclude)src/secp256k1/" ":(exclude)src/univalue/" | wc -l)
$ bc <<< "scale=4; ${LOC_AFTER}/${LOC_BEFORE}"
.9985
```
:-)
Tree-SHA512: 9588a3cd795a89ef658b8ee7323865f57723cb4ed9560c21de793f82d35e2835059e7d6d0705e99e3d16bf6b2a444b4bf19568d50174ff3776caf8a3168f5c85
fa926ec24f rpc: Mention all output types in decodescript doc (MarcoFalke)
fa3caa1666 rpc: decodescript use IsValidNumArgs over hardcoded check (MarcoFalke)
faad33ff15 rpc: Clarify decodescript RPCResult doc (MarcoFalke)
Pull request description:
* Remove `"hex"` from the decodescript RPCResult doc
* Add `"segwit`" to the doc
Follow up to a6099ef319 and 4f933b3d23
ACKs for commit fa926e:
ryanofsky:
utACK fa926ec24f. Only change since last review is listing possible output types in the help string using a new `GetAllOutputTypes` function
Tree-SHA512: e6ecc563d04769942567118d50188467bf64ceb276ba6268928d469e8f06621f2ca1ae1e555d3daa6ec22a615ee259bb31c4141c19818d0f53fb6c529b18381b
43ae1e96ed release: Update the Windows Codesigning certificate (Cory Fields)
Pull request description:
Thanks to @gwillen for paying and @jonasschnelli for setting up the small company needed for signing!
This updates the Windows codesigning certificate to replace our expired one.
Testing showed that Windows 7 had trouble finding the path to a trusted CA. I am not sure if this will work better with more recent Windows versions, but because the previous cert is expired, this can only be an improvement.
This needs feedback after rc3.
ACKs for commit 43ae1e:
laanwj:
utACK 43ae1e96ed
achow101:
utACK 43ae1e96ed
Tree-SHA512: 2e70591d2deeac5f6e73d73474ccefba5db6cfd9004c40a56c7c2d8300b67ccee4916a04f43e0b75796cc1089d111e30276819e0bfb954c63c4760b8d96423a4
529c1ae4a0 [tests] Add test for wallet rebroadcasts (John Newbery)
Pull request description:
The existing wallet_resendwallettransactions.py test only tests the
resendwallettransactions RPC. It does not test whether transactions are
actually rebroadcast, or whether the rebroadcast logic is called on a
timer.
Update the test to not use the resendwallettransactions RPC and test
that transactions are resent on a timer.
ACKs for commit 529c1a:
MarcoFalke:
re-utACK 529c1ae4a0
Tree-SHA512: 7341e7dd07cdc8ecbc08b1949121824148d2b58133a8e298ecdc5b7555713df3cecffb49854443cef9f033ef847cbf329e879a3bf57ab4e1fc733be432e9f718
The existing wallet_resendwallettransactions.py test only tests the
resendwallettransactions RPC. It does not test whether transactions are
actually rebroadcast, or whether the rebroadcast logic is called on a
timer.
This commit updates the test to not use the resendwallettransactions RPC and
test that transactions are rebroadcast on a timer.
fa1ad200d3 doc: Add release notes for 15620 (MarcoFalke)
fa96d76421 rpc: Uncouple rpcs from maxTxFee global (MarcoFalke)
fa965e03c7 rpc: Use IsValidNumArgs over hardcoded size checks (MarcoFalke)
Pull request description:
This makes the rpcs a bit more stateless by falling back to their own default max fee instead of the global maxTxFee.
A follow up pull request will move `-maxtxfee` to the wallet.
See also related discussions:
* `-maxtxfee` should not be used by both node and wallet #15355
* [RFC] Long term plan for wallet command-line args #13044
ACKs for commit fa1ad2:
jnewbery:
utACK fa1ad200d3
Empact:
utACK fa1ad200d3
jnewbery:
utACK fa1ad200d3
promag:
utACK fa1ad20.
Tree-SHA512: c9cf0b54cd30ff3ab0d090b072cc38fcbb2840bc6ad9a9711995333bc927d2500aece6b5a60e061666eca5ed72b70aa318d21e51eb15ee0106b41f5b6e4e1adf
fa1c073154 contrib: gh-merge: Include review comments in merge commit (MarcoFalke)
Pull request description:
This includes all up-to-date ACKs in the merge commit for reference
Tree-SHA512: 32c9352d884f9ecf94940f50f2921fc9fc026083c120f54d0651a41814872e852aee8d0c4ad5bcd03292329f05d76fcb7bac11741e1dd3bf417211a186005afb
e9d5e97561 Poly1305: tolerate the intentional unsigned wraparound in poly1305.cpp (Jonas Schnelli)
b34bf302f2 Add Poly1305 bench (Jonas Schnelli)
03be7f48fa Add Poly1305 implementation (Jonas Schnelli)
Pull request description:
This adds a currently unused Poly1305 implementation including test vectors from RFC7539.
Required for BIP151 (and related to #15512).
Tree-SHA512: f8c1ad2f686b980a7498ca50c517e2348ac7b1fe550565156f6c2b20faf764978e4fa6b5b1c3777a16e7a12e2eca3fb57a59be9c788b00d4358ee80f2959edb1
Refactor the analyzepsbt RPC into (1) an AnalyzePSBT function, which returns
its output as a new strongly-typed PSBTAnalysis struct, and (2) a thin wrapper
which converts the struct into a UniValue for RPC use.
