aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2020-11-19Update univalue subtreeMarcoFalke
2020-11-19Squashed 'src/univalue/' changes from 98261b1e7b..98fadc0909MarcoFalke
98fadc0909 Merge #24: Push bool into array correctly 5f03f1f39a Push bool into array correctly git-subtree-dir: src/univalue git-subtree-split: 98fadc090984fa7e070b6c41ccb514f69a371c85
2020-11-19Merge #18531: rpc: remove deprecated CRPCCommand constructorMarcoFalke
faaf9c58e4aa809019d4ca12747dd47411988e37 remove CRPCCommand constructor that takes rpcfn_type function pointer (MarcoFalke) fa19bb2cd8c575593583138a84e6bb3444d6196d remove dead rpc code (MarcoFalke) Pull request description: Remove the CRPCCommand arguments, now that they are asserted to be equal and thus redundant ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue #18607 * The changes itself fixed bug #19250 ACKs for top commit: fjahr: tested ACK faaf9c58e4aa809019d4ca12747dd47411988e37 promag: Tested ACK faaf9c58e4aa809019d4ca12747dd47411988e37. ryanofsky: Code review ACK faaf9c58e4aa809019d4ca12747dd47411988e37. Two obviously good simplifications. Tree-SHA512: 5de3b440f7b2ed2c3e86655d4f0e2e5df9c67e8ce3c7817d5ea5311d1a38690f2f3e28fab41aad6936be9fc884326d037e5f19e85d4d2fe281474dada13911ee
2020-11-19Merge #15710: wallet: Catch ios_base::failure specificallyWladimir J. van der Laan
7486e2771e7b5d6fa84df6e954be76350c84e220 Tests: Unit test related to WalletDB ReadKeyValue (Bushstar) 32def8d1c29e0855fe5429687acabd2f29119316 Catch ios_base::failure specifically (Peter Bushnell) Pull request description: In https://github.com/bitcoin/bitcoin/pull/2950 a hash of the pubkey and private was added to speed up key import, this was made backwards compatible by reading the hash in a try block with an ellipses catch all in case the hash was not present. CDataStream::read() specifically throws std::ios_base::failure, backwards compatibility expects only that error to be thrown, if something else gets thrown we should not be catching it. The change in this commit is to catch that exception only. If any other exception is thrown other than std::ios_base::failure it will be caught by the wider try block and an error written to the log and/or console. CDataStream::read() throwing std::ios_base::failure. https://github.com/bitcoin/bitcoin/blob/2c364fde423e74b4e03ebcff4582a9db7a6c4e4b/src/streams.h#L191 Wider catch statements that pick up all others exceptions other than ios_base::failure. https://github.com/bitcoin/bitcoin/blob/2c364fde423e74b4e03ebcff4582a9db7a6c4e4b/src/wallet/walletdb.cpp#L425 https://github.com/bitcoin/bitcoin/blob/2c364fde423e74b4e03ebcff4582a9db7a6c4e4b/src/wallet/walletdb.cpp#L430 ACKs for top commit: laanwj: Code review ACK 7486e2771e7b5d6fa84df6e954be76350c84e220 Tree-SHA512: 5364bf935af8ec603bf5b8fef8c23b5cdaa4fe3506090cff988413221f2eaa99f7a91929afb42a35f8881ce2328744a0d32052da51ca0a5b2e65b6809e97f604
2020-11-19Merge #20145: contrib: add getcoins.py script to get coins from (signet) faucetWladimir J. van der Laan
e9c8e6eea2dde6ccd5f685956392ee70c8cfefb7 doc: add contrib/signet readme (Karl-Johan Alm) 355d0c4f6b8a7687a3940a2d90d66b08e560bfa7 contrib: add getcoins.py script to get coins from (signet) faucet (Karl-Johan Alm) Pull request description: This adds a small python script that can be used to fetch Signet coins from the default (or custom) faucet. ACKs for top commit: laanwj: Code and documentation review ACK e9c8e6eea2dde6ccd5f685956392ee70c8cfefb7 Tree-SHA512: 9aaeb96bf0c636a38e2dbe4cfc8b3ef907b1c05d0b782ee51223014952e07ce45a071c7e99aa9aa7700196a67f8a47d74d13e5e8d6890b9be503acd2bacd4d4f
2020-11-19Merge #20358: src/randomenv.cpp: fix build on uclibcWladimir J. van der Laan
330cb33985d0ce97c20f4a0f0bbda0fbffe098d4 src/randomenv.cpp: fix build on uclibc (Fabrice Fontaine) Pull request description: Check for HAVE_STRONG_GETAUXVAL or HAVE_WEAK_GETAUXVAL before using getauxval to avoid a build failure on uclibc Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> ACKs for top commit: laanwj: Code review ACK 330cb33985d0ce97c20f4a0f0bbda0fbffe098d4 Tree-SHA512: 94fbbdb0e859f0220d64b2d04565f575b410327f080125fec7fb74205d0bea0e8133561c83a696033d6dc377871133871b72c1aad19aca61e972ce67e0fdf707
2020-11-19Merge #20067: refactor: remove use of boost::algorithm::replace_firstWladimir J. van der Laan
6f4e3936461d0893250e7684928339f6cedf4d0c refactor: remove use of boost::algorithm::replace_first (Sebastian Falbesoner) Pull request description: As discussed in #19851 (https://github.com/bitcoin/bitcoin/pull/19851#issuecomment-685424702), this trivial PR substitutes the (only) use of `boost::algorithm::replace_first` by a direct implementation. ACKs for top commit: laanwj: Code review ACK 6f4e3936461d0893250e7684928339f6cedf4d0c Tree-SHA512: 2ef06498e19f864a4cbae10e8d1905e3440a2d1e8e5aae83de7597c23cdab92b4612d7fa1efbc49016e530debd127d1d50531c60ff159dbea0deaa8c836a2bfb
2020-11-19Merge #19968: doc: clarify CRollingBloomFilter size estimateWladimir J. van der Laan
d9141a0002bb508b2e94e206a1bd28ef8f97ffde doc: clarify CRollingBloomFilter size estimate (Anthony Towns) Pull request description: Based on #19130, this change improves the comment for `CRollingBloomFilter` in `bloom.h`: - Give examples to illustrate the heuristic "1.8 bytes per element per factor 0.1 of false positive rate" - Add some Python code which can be copy/pasted for convenient filter size calculation (in an interpreter) - Reconcile the newly added code with the existing approximation ACKs for top commit: laanwj: ACK d9141a0002bb508b2e94e206a1bd28ef8f97ffde Tree-SHA512: e7138b3c531883a750ead06368975c750863fde7ef6f2633b137eca011079226e9205316217322014399fba05a48f294c788dd700bb7d479c58fe1f23e40419f
2020-11-19Merge #20333: build: remove native_biplist dependencyWladimir J. van der Laan
7087440894a9daa7de806c5aa42d83ad60759c65 depends: native_ds_store 1.3.0 (fanquake) Pull request description: `ds_store` [now takes advantage](https://github.com/al45tair/ds_store/commit/36fb60794029b8556ee13693e7020a7e13d0c04b) of Pythons ability to decode binary [plists](https://docs.python.org/3/library/plistlib.html) (since 3.4), so we can drop its biplist dependency. The call to `biplist.Data()` in `custom_dsstore.py` doesn't seem to do anything, and from what I can tell can just be removed. i.e: ```diff diff --git a/contrib/macdeploy/custom_dsstore.py b/contrib/macdeploy/custom_dsstore.py index dc1c1882d..e475bc6c3 100755 --- a/contrib/macdeploy/custom_dsstore.py +++ b/contrib/macdeploy/custom_dsstore.py @@ -47,6 +47,7 @@ alias.volume.disk_image_alias.target.filename = package_name_ns + '.temp.dmg' alias.volume.disk_image_alias.target.carbon_path = 'Macintosh HD:Users:\x00bitcoinuser:\x00Documents:\x00bitcoin:\x00bitcoin:\x00' + package_name_ns + '.temp.dmg' alias.volume.disk_image_alias.target.posix_path = 'Users/bitcoinuser/Documents/bitcoin/bitcoin/' + package_name_ns + '.temp.dmg' alias.target.carbon_path = package_name_ns + ':.background:\x00background.tiff' +assert(biplist.Data(alias.to_bytes()) == alias.to_bytes()) icvp['backgroundImageAlias'] = biplist.Data(alias.to_bytes()) ds['.']['icvp'] = icvp ``` ACKs for top commit: laanwj: ACK 7087440894a9daa7de806c5aa42d83ad60759c65 Tree-SHA512: 8ba3cf561937efe4a3daae8b0cb4de3bf9e425b3a9244161b09d94ee2b1bd4c3e21315fa70e495b19a052aabdc1731b3b6f346b63272d72d2762ced83237d02f
2020-11-19Merge #20359: depends: Various config.site.in improvements and lintingWladimir J. van der Laan
46756a69877ab7d56f3b05f8c5a8899eb24d9ffd depends: Fix PYTHONPATH setting in config.site.in (Carl Dong) 618cbd2c1a630a60bed9212718dce78fe5f50108 lint: Also lint files with shellcheck directive (Carl Dong) 6c7e8f067dcf4c5d793d162aeb84f074f9a14664 depends: Allow relative CONFIG_SITE path env var (Carl Dong) Pull request description: This changeset: 1. Allows the `CONFIG_SITE` env var to be a relative path rather than requiring an absolute one 2. Enables linting of the `config.site.in` file with `shellcheck` in our linting scripts 3. Sets the `PYTHONPATH` var sensibly in `config.site.in` Please see commit messages for more details ACKs for top commit: laanwj: ACK 46756a69877ab7d56f3b05f8c5a8899eb24d9ffd Tree-SHA512: 744089b9f6e5604e56466d9a3e64563f9183a70f7e300ac9ae6248f0f17c0b53fe28a2c41d43c5ffe5da825f53c2ca21f21aacba0579442da3056fb0c4b81454
2020-11-19Merge #20288: script, doc: contrib/seeds updatesWladimir J. van der Laan
961f148cb1b4caab86b4354357999e03433b04b1 doc: update contrib/seeds/README dnspython installation info (Jon Atack) dd7b5f46d85401254630abf6976f59b5b8eed181 script: fix deprecation warning in makeseeds.py (Jon Atack) Pull request description: Seen while reviewing #20237. 1. Fix a deprecation warning in `contrib/seeds/makeseeds.py` ``` makeseeds.py:139: DeprecationWarning: please use dns.resolver.resolve() instead asn = int([x.to_text() for x in dns.resolver.query('.'.join( ``` - Per https://dnspython.readthedocs.io/en/latest/whatsnew.html, `dns.resolver.query()` was deprecated in `dnspython` version 2.0.0. - See https://dnspython.readthedocs.io/en/latest/resolver-class.html for more info on the resolver class. 2. Update the `dnspython` dependency installation instructions in `contrib/seeds/README` - The markdown rendering can be seen here: https://github.com/jonatack/bitcoin/tree/contrib-seeds-fixups/contrib/seeds ACKs for top commit: laanwj: code review ACK 961f148cb1b4caab86b4354357999e03433b04b1 Tree-SHA512: f9c4f318a1a0d35b8de147d24b72c534a1f58eece31e7cfa00b4149a63b6a618d8ca0312f52fd8056f3c645cf2ee68574ca02319fddffdad919a70cd33395d33
2020-11-19Merge #20413: build: Require C++17 compilerfanquake
fac71987281077aed7f79dce99f4eb3e8a91916a Use std::make_unique (MarcoFalke) faaee810e62d796d66bfb2bc4e53c8b4c8104d13 build: Require C++17 compiler (MarcoFalke) Pull request description: Developers have been compiling with C++17 for a few months now (fuzz tests and the msvc build have it even enabled by default). According to #16684, the 22.0 release shall be compiled with C++17 enabled. This only sets the build flag, any other changes need more discussion and can be done later. ACKs for top commit: elichai: utACK fac71987281077aed7f79dce99f4eb3e8a91916a hebasto: ACK fac71987281077aed7f79dce99f4eb3e8a91916a, I've locally compiled on ARM 32bit SBC without GUI. fanquake: ACK fac71987281077aed7f79dce99f4eb3e8a91916a Tree-SHA512: 53eb40ba5d496376a2d2cf16e2000bef36616cc2a3696c3ec59a5366e41fa8b872817a7ca21751f030f9c1efb339dfa63cc655eaa5199b9a3d2e52c2de0ccb29
2020-11-18Use std::make_uniqueMarcoFalke
2020-11-18build: Require C++17 compilerMarcoFalke
2020-11-18Merge #20414: doc: Remove generated manual pages from master branchMarcoFalke
daf1ebf0b1c2020169d75fc52c3be87207693a04 doc: Remove generated manual pages from master branch (Wladimir J. van der Laan) Pull request description: Replace the generated manual pages on the master branch with placeholders with instructions. The master branch is too much in flux for anything generated from command output to be kept up to date, and having pages from versions ago looks silly. We can't remove them completely because `make dist` relies on the files being present. Resolves #20062. ACKs for top commit: MarcoFalke: review ACK daf1ebf0b1c2020169d75fc52c3be87207693a04 RiccardoMasutti: ACK daf1ebf Tree-SHA512: fdd0bb777ddf66c663e4f6b00d206b3f6283ae89028e00b38d16e68a224a781e9245a25edf6f3326f9252ae2ef7c2950c591667c56d4e9d10a16fc50a6c358f4
2020-11-18Merge #20418: doc: clean out release notes post branch-offMarcoFalke
8a715a6b17a8c10369dcfad735b4362bef0c326c build: Bump gitian descriptors to 0.22 (fanquake) dc5a35a5072b273ce2be5b2110bacf298150a725 doc: clean out release notes post branch-off (fanquake) Pull request description: Usually done during a version bump. i.e: https://github.com/bitcoin/bitcoin/commit/d84c9aa25d8b86d7b3ff2205560986f012da727a. ACKs for top commit: MarcoFalke: review ACK 8a715a6b17a8c10369dcfad735b4362bef0c326c 👜 Tree-SHA512: e185b4b51f52014777905c346185e1d6cfd147ce5431532eee4e188a70fc942da389d93cf61ccc9466126ec05a4d3896cab439029cb7ffc47b67e8af03b6b0a4
2020-11-18build: Bump gitian descriptors to 0.22fanquake
2020-11-18doc: clean out release notes post branch-offfanquake
2020-11-18Merge #20408: CConnman: move initialization to declarationfanquake
9d09132be4ff99f98ca905c342347d5f35f13350 CConnman: initialise at declaration rather than in Start() (Anthony Towns) Pull request description: Ensure nMaxOutboundTotalBytesSentInCycle and nMaxOutboundCycleStartTime are initialized even if CConnman::Start() is not called. Prevents failures in test/fuzz/connman when run under valgrind. ACKs for top commit: practicalswift: ACK 9d09132be4ff99f98ca905c342347d5f35f13350: patch looks correct! MarcoFalke: review ACK 9d09132be4ff99f98ca905c342347d5f35f13350 , checked that we call Start only once and in the same scope where connman is constructed (AppInitMain) 💸 jnewbery: Code review ACK 9d09132be4 Tree-SHA512: 1c6c893e8c616a91947a8cc295b0ba508af3ecfcdcd94cdc5f95d808cc93c6d1a71fd24dcc194dc583854e9889fb522ca8523043367fb0263370fbcab08c6aaa
2020-11-18Merge #19905: Remove dead CheckForkWarningConditionsOnNewForkMarcoFalke
fa7eed5be704ccdbdce5c9aedb953dd9c8b30446 doc: Clarify that vpindexToConnect is in reverse order (MarcoFalke) fa62304c9760f0de9838e56150008816e7a9bacb Remove dead CheckForkWarningConditionsOnNewFork (MarcoFalke) Pull request description: The function has several code and logic bugs, which prevent it from working at all: * `vpindexToConnect.back()` is passed to `CheckForkWarningConditionsOnNewFork`, which is the earliest connected block (least work block), *not* the new fork tip * `ActivateBestChainStep` will never try to connect a block that descends from an invalid block, so the invalid fork will only ever be of height 1, never hitting the 7 block minimum condition Instead of dragging the dead and wrong code around through every change in validation, remove it. In the future it could make sense to add a fork detection somewhere outside of the `ActivateBestChainStep` logic (maybe net_processing). ACKs for top commit: jnewbery: utACK fa7eed5be704ccdbdce5c9aedb953dd9c8b30446 fjahr: Code review ACK fa7eed5be704ccdbdce5c9aedb953dd9c8b30446 glozow: utACK https://github.com/bitcoin/bitcoin/commit/fa7eed5be704ccdbdce5c9aedb953dd9c8b30446 I see that it's dead code Tree-SHA512: 815bdbac7c1eb5b7594b0866a2dbd3c7619797afaadb03a5269fb96739ffb83b05b8e4f7c1e68d48d7886132dd0b12c14c3fb4ee0e72de1074726050ed203e1a
2020-11-18Merge bitcoin-core/gui#109: wallet: Remove unused AskPassphraseDialog::DecryptMarcoFalke
4146a31ccbb012ff552f303113979b48c086532b qt, wallet: Drop unused parameter in WalletModel::setWalletEncrypted (Hennadii Stepanov) f886a20b02094d657ddb3d792d561d50f2107f07 qt, wallet: Drop unused parameter in Wallet{Frame|View}::encryptWallet (Hennadii Stepanov) 6e950118a31fd6a85026d934fc6adb6255e47e23 qt, wallet: Remove unused AskPassphraseDialog::Decrypt (Hennadii Stepanov) Pull request description: Grabbed from #42 with an additional commit. Fix #1. ACKs for top commit: MarcoFalke: ACK 4146a31ccbb012ff552f303113979b48c086532b promag: Code review ACK 4146a31ccbb012ff552f303113979b48c086532b. Tree-SHA512: 6070d8995525af826ad972cf1b8988ff98af0528eef285a07ec7ba0e2e92a7a6173a19dc371de94d4b437fa10f7921166e45a081de6ed2f4306e6502aafc94ee
2020-11-18Merge bitcoin-core/gui#118: Remove BDB version from the Information tabMarcoFalke
e4fc45a0115eea8ad7ad7505dfcfd8cbe50b96da gui: Remove BDB version from the Information tab (Hennadii Stepanov) Pull request description: Master (67d4643a1a763b893ee69f6bbbf59ef1f2cb0d51): ![DeepinScreenshot_select-area_20201027161350](https://user-images.githubusercontent.com/32963518/97313812-b90cea80-186f-11eb-8fec-781b0724bd9c.png) This PR: ![DeepinScreenshot_select-area_20201027161449](https://user-images.githubusercontent.com/32963518/97313883-c924ca00-186f-11eb-8f82-ebb9f68414b1.png) ACKs for top commit: achow101: ACK e4fc45a0115eea8ad7ad7505dfcfd8cbe50b96da jonasschnelli: utACK e4fc45a0115eea8ad7ad7505dfcfd8cbe50b96da Sjors: utACK e4fc45a promag: ACK e4fc45a0115eea8ad7ad7505dfcfd8cbe50b96da. jarolrod: Tested ACK e4fc45a Tree-SHA512: 3660b78607a4dc364b7cb2bc74ccd8b57acd7a16084151e58945e1a92a9af97a37edb33e94a87067fabfe9ce1c45be11fab270bc146f742a2f1010530470c433
2020-11-18doc: Remove generated manual pages from master branchWladimir J. van der Laan
Replace the generated manual pages on the master branch with placeholders with instructions. The master branch is too much in flux for anything generated from command output to be kept up to date, and having pages from versions ago looks silly. We can't remove them completely because `make dist` relies on the files being present. Resolves #20031.
2020-11-18Merge #19504: Bump minimum python version to 3.6MarcoFalke
97c738ff1b592270491551cc0a43472d244ffbb0 [tests] Recommend f-strings for formatting, update feature_block to use them (Anthony Towns) 8ae9d314e9af7bcce1e8bc52f0317b9d565109bf Bump minimum python version to 3.6 (Anthony Towns) Pull request description: Python 3.5 has reached [end-of-life](https://devguide.python.org/#status-of-python-branches) as of September 2020, and 3.6 has some moderately nice [features](https://docs.python.org/3/whatsnew/3.6.html): - `f'x = {x}'` as an alternative to `'x = {}'.format(x)` format strings (cf https://github.com/bitcoin/bitcoin/pull/13718#issuecomment-406591027) - underscore separators for large numbers, like `1_234_567` - improvements to async - improvements to typing module Note that 3.6 is not available in xenial (16.04), but is available in bionic (18.04), while focal (20.04) has 3.8. CentOS 7 and 8 have 3.6.8, Debian stable has 3.7.3, and [gentoo and arch already had 3.6 and 3.7 in 2018](https://github.com/bitcoin/bitcoin/pull/14954#issuecomment-447118707). ACKs for top commit: MarcoFalke: re-ACK 97c738ff1b Tree-SHA512: ec7fce68845edde4d61a42de12c065fd49e5217311a6fda1323206f091a0afd50f293645dffc27d420127e4e5deb864e953f1b67eff735a0dfbbedd7899a9d60
2020-11-18build: Bump master version to 0.21.99Wladimir J. van der Laan
Tree-SHA512: 94c258b234b2412d92f312a1b38adf17249664a9e3e321de0ff683b59a48cee192cd42da5220df0726a782d98776610f4420534b3a1c51f4cf4a0180d5835622
2020-11-18CConnman: initialise at declaration rather than in Start()Anthony Towns
Ensure nMaxOutboundTotalBytesSentInCycle and nMaxOutboundCycleStartTime are initialized even if CConnman::Start() is not called.
2020-11-17Merge #20401: qt: Pre-splitoff translations updatefanquake
bb6441b7a4619dd11029e27126c0d727a8bdf2d2 qt: Pre-splitoff translations update (Wladimir J. van der Laan) Pull request description: 0.21 split-off should be near now. Let's do one final translations update just before the split-off. (Hopefully it won't take too long, but might want to keep this open to be the last thing merged) ACKs for top commit: hebasto: ACK bb6441b7a4619dd11029e27126c0d727a8bdf2d2 MarcoFalke: ACK bb6441b7a4619dd11029e27126c0d727a8bdf2d2 (checked that only changes are translation changes in `src/qt`) Tree-SHA512: 3273246923d3020e1f7ae46cbb59f1ed45a35acb5e1582b55486c5723f5aa1e5809fe2fd87b1ac34d308eef2902e621d0ace97181a044262b2c8f002bf50daac
2020-11-17Merge #20305: wallet: introduce fee_rate sat/vB param/optionMarcoFalke
05e82d86b09d914ebce05dbc92a7299cb026847b wallet: override minfee checks (fOverrideFeeRate) for fee_rate (Jon Atack) 9a670b4f07a6140de809d73cbd7f3e614eb6ea74 wallet: update sendtoaddress, send RPC examples with fee_rate (Jon Atack) be481b72e24fb6834bd674cd8daee67c6938b42d wallet: use MIN_RELAY_TX_FEE in bumpfee help (Jon Atack) 449b730579566459e350703611629e63e54657ed wallet: provide valid values if invalid estimate mode passed (Jon Atack) 6da3afbaee5809ebf6d88efaa3958c505c2d71c7 wallet: update remaining rpcwallet fee rate units to BTC/kvB (Jon Atack) 173b5b5fe07d45be5a1e5bc7a5df996f20ab1e85 wallet: update fee rate units, use sat/vB for fee_rate error messages (Jon Atack) 7f9835a05abf3e168ad93e7195cbaa4bf61b9b07 wallet: remove fee rates from conf_target helps (Jon Atack) b7994c01e9a3251536fe6538a22f614774eec82d wallet: add fee_rate unit warnings to bumpfee (Jon Atack) 410e471fa42d3db04e8879c71f8c824dcc151a83 wallet: remove redundant bumpfee fee_rate checks (Jon Atack) a0d495747320c79b27a83c216dcc526ac8df8f24 wallet: introduce fee_rate (sat/vB) param/option (Jon Atack) e21212f01b7c41eba13b0479b252053cf482bc1f wallet: remove unneeded WALLET_BTC_KB_TO_SAT_B constant (Jon Atack) 6112cf20d43b0be34fe0edce2ac3e6b27cae1bbe wallet: add CFeeRate ctor doxygen documentation (Jon Atack) 3f7279161347543ce4e997d78ea89a4043491145 wallet: fix bug in RPC send options (Jon Atack) Pull request description: This PR builds on #11413 and #20220 to address #19543. - replace overloading the conf_target and estimate_mode params with `fee_rate` in sat/vB in the sendtoaddress, sendmany, send, fundrawtransaction, walletcreatefundedpsbt, and bumpfee RPCs - allow non-actionable conf_target value of `0` and estimate_mode value of `""` to be passed to use `fee_rate` as a positional argument, in addition to as a named argument - fix a bug in the experimental send RPC described in https://github.com/bitcoin/bitcoin/pull/20220#discussion_r513789526 where args were not being passed correctly into the options values - update the feerate error message units for these RPCs from BTC/kB to sat/vB - update the test coverage, help docs, doxygen docs, and some of the RPC examples - other changes to address the excellent review feedback See this wallet meeting log for more context: http://www.erisian.com.au/bitcoin-core-dev/log-2020-11-06.html#l-309 ACKs for top commit: achow101: re-ACK 05e82d8 MarcoFalke: review ACK 05e82d86b0 did not test and found a few style nits, which can be fixed later 🍯 Xekyo: tACK 05e82d86b09d914ebce05dbc92a7299cb026847b Sjors: utACK 05e82d86b09d914ebce05dbc92a7299cb026847b Tree-SHA512: a4ee5f184ada53f1840b2923d25873bda88c5a2ae48e67eeea2417a0b35154798cfdb3c147b05dd56bd6608a784e1b91623bb985ee2ab9ef2baaec22206d0a9c
2020-11-17Merge bitcoin-core/gui#96: Slight improve create wallet dialogMarcoFalke
ac64cec4ce35cb7e6fbf3678c1ffeac0137791ae gui: create wallet: add advanced section (Sjors Provoost) c99d6f644aa45d1bd929790f23a36d0dd7c29004 gui: create wallet: name placeholder (Sjors Provoost) 5bff82540b90d899ceac6390c008d653e6b665c3 [gui] create wallet: smarter checkbox toggling (Sjors Provoost) Pull request description: Previously only users who needed a second wallet had to use to the create wallet dialog. With the merge of https://github.com/bitcoin/bitcoin/pull/15454 now all new users have to. I don't think it was user-friendly enough for that. <img width="403" alt="Schermafbeelding 2020-09-18 om 09 41 44" src="https://user-images.githubusercontent.com/10217/93574129-52ef9680-f998-11ea-9a6f-31144f66d3bf.png"> This PR makes a few simple improvements so that new users don't have to think too much: <img width="369" alt="Schermafbeelding 2020-10-15 om 16 45 22" src="https://user-images.githubusercontent.com/10217/96145959-0c914700-0f06-11eb-9526-cf447d841d7a.png"> It's lightly inspired by #77. It would be better if those changes made it into the upcoming release, but this PR is a good start imo. * wallet encryption is no longer checked by default, because such a change in the default needs a separate discussion (fwiw, I suspect it increases the number of users losing access to coins) * watch-only and descriptor wallet stuff is moved to advanced, so new users know they can safely ignore these check boxes * bonus: when you click on "disable private keys" it disables encrypt wallet and checks blank wallet * label changes: see screenshot * tooltip changes: see code diff Note that a blank wallet name isn't allowed in the dialog; I haven't addressed that. _Update 2020-10-30_, dropped the new strings for now: <img width="450" alt="Schermafbeelding 2020-10-30 om 11 26 55" src="https://user-images.githubusercontent.com/10217/97694591-1b99fc80-1aa3-11eb-8b85-e19f1ad5add4.png"> ACKs for top commit: fjahr: Tested ACK ac64cec4ce35cb7e6fbf3678c1ffeac0137791ae jonatack: re-ACK ac64cec4ce35cb7e6fbf3678c1ffeac0137791ae, per `git diff d393708 ac64cec` only change since my last review is improving the placeholder from "MyWallet" to "Wallet" and dropping the last commit. Tested creating a dozen wallets in signet with different combinations of options and then verifying/comparing their characteristics in the console with getwalletinfo. My remaining caveats are (1) the need for less user surprise by either (a) improving the user info or (b) with less auto-(un)selecting as mentioned in https://github.com/bitcoin-core/gui/pull/96#issuecomment-727017409 and (2) I prefer the "Encrypt private keys" and "Watch-only" wording and descriptions below over the current ones; hopefully these can be addressed in a follow-up. hebasto: re-ACK ac64cec4ce35cb7e6fbf3678c1ffeac0137791ae ryanofsky: Code review ACK ac64cec4ce35cb7e6fbf3678c1ffeac0137791ae. Only changes since last review are tweaking placeholder text and dropping "allow nameless" commit Tree-SHA512: a25f84eb66ee4f99af441d73e33928df9d9cf592177398ef48f0037f5913699e47a162cf1301c83b34501546d43ff4ae12607fd078c5c03b92f573bf7604a9f2
2020-11-17Merge #20139: Wallet: do not return warnings from UpgradeWallet()MarcoFalke
963696288955dc31b3a4fd136bfb791a9d99755b [upgradewallet] removed unused warning param (Sishir Giri) Pull request description: The `warning` variable was unused in `upgradewallet` so I removed it ACKs for top commit: practicalswift: ACK 963696288955dc31b3a4fd136bfb791a9d99755b: diff looks correct MarcoFalke: review ACK 963696288955dc31b3a4fd136bfb791a9d99755b jonatack: ACK 963696288955dc31b3a4fd136bfb791a9d99755b Tree-SHA512: 1d63186ce1e05e86a778340f2d7986c2cee1523de0a11cea39e8d148ac7ee26c49741dfa302b5c1cd1c8d74e67c1f9baee2763720c2d850b57da9a3fdce24565
2020-11-17Merge #20346: script: modify security-check.py to use "==" instead of "is" ↵fanquake
for literal comparison b6121edf70a8d50fd16ddbba0c3168e5e49bfc2e swapped "is" for "==" in literal comparison (Tyler Chambers) Pull request description: In Python 3.8+ literal comparisons using "is" instead of "==" produce a SyntaxWarning [source](https://docs.python.org/3.8/whatsnew/3.8.html#changes-in-python-behavior). I checked the entire devtools directory, this seems to be the only occurrence. This is a small fix, but removes the SyntaxWarning. Fixes: #20338 ACKs for top commit: hebasto: re-ACK b6121edf70a8d50fd16ddbba0c3168e5e49bfc2e, only squashed since my [previous](https://github.com/bitcoin/bitcoin/pull/20346#pullrequestreview-525934568) review. practicalswift: re-ACK b6121edf70a8d50fd16ddbba0c3168e5e49bfc2e: patch still looks correct theStack: utACK b6121edf70a8d50fd16ddbba0c3168e5e49bfc2e Tree-SHA512: 82a43495d6552fbaa3b02b58f0930b049d27aa937fe44b47714e3c059f844cc494de20674557371cbccf24fb8873ecb7376fb965ae326847eed2b855ed2d59c6
2020-11-17Merge #20405: p2p: avoid calculating onion address checksum when version is ↵fanquake
not 3 d355a302d9b7e4aaac04edaa0671ced3b3eaef45 Break circuit earlier (lontivero) Pull request description: Currently when parsing an onion v3 address the pubic key checksum is calculated in order to compare it with the received address checksum. However this step is not necessary if the address version byte is not 3, in which case the method can return with false immediately. ACKs for top commit: jonatack: ACK d355a302d9b7e4aaac04edaa0671ced3b3eaef45 practicalswift: ACK d355a302d9b7e4aaac04edaa0671ced3b3eaef45 -- patch looks correct hebasto: ACK d355a302d9b7e4aaac04edaa0671ced3b3eaef45, I have reviewed the code and it looks OK, I agree it can be merged. sipa: utACK d355a302d9b7e4aaac04edaa0671ced3b3eaef45 Tree-SHA512: 9e4506793b7f4a62ce8edc41a260a8c125ae81ed2f90cd850eb2a9214d323c446edc7586c7b0590dcbf3aed5be534718b77bb19c45b48f8f52553d32a3663a65
2020-11-16[upgradewallet] removed unused warning paramSishir Giri
2020-11-16Break circuit earlierlontivero
There is no need to calculate the full checksum for an Tor v3 onion address if the version byte is not the expected one.
2020-11-16qt: Pre-splitoff translations updateWladimir J. van der Laan
2020-11-16Merge #18836: wallet: upgradewallet fixes and additional testsWladimir J. van der Laan
5f9c0b6360215636cfa62a70d3a70f1feb3977ab wallet: Remove -upgradewallet from dummywallet (MarcoFalke) a314271f08215feba53ead27096ac7fda34acb3c test: Remove unused wallet.dat (MarcoFalke) bf7635963c03203e7189ddaa56c6b086a0108cbf tests: Test specific upgradewallet scenarios and that upgrades work (Andrew Chow) 4b418a9decc3e855ee4b0bbf9e61121c8e9904e5 test: Add test_framework/bdb.py module for inspecting bdb files (Andrew Chow) 092fc434854f881330771a93a1280ac67b1d3549 tests: Add a sha256sum_file function to util (Andrew Chow) 0bd995aa19be65b0dd23df1df571c71428c2bc32 wallet: upgrade the CHDChain version number when upgrading to split hd (Andrew Chow) 8e32e1c41c995e832e643f605d35a7aa112837e6 wallet: remove nWalletMaxVersion (Andrew Chow) bd7398cc6258c258e9f4411c50630ec4a552341b wallet: have ScriptPubKeyMan::Upgrade check against the new version (Andrew Chow) 5f720544f34dedf75b063b962845fa8eca604514 wallet: Add GetClosestWalletFeature function (Andrew Chow) 842ae3842df489f1b8d68e67a234788966218184 wallet: Add utility method for CanSupportFeature (Andrew Chow) Pull request description: This PR cleans up the wallet upgrade mechanism a bit, fixes some probably bugs, and adds more test cases. The `nWalletMaxVersion` member variable has been removed as it made `CanSupportFeature` unintuitive and was causing a couple of bugs. The reason this was introduced originally was to allow a wallet upgrade to only occur when the new feature is first used. While this makes sense for the old `-upgradewallet` option, for an RPC, this does not quite make sense. It's more intuitive for an upgrade to occur if possible if the `upgradewallet` RPC is used as that's an explicit request to upgrade a particular wallet to a newer version. `nWalletMaxVersion` was only relevant for upgrades to `FEATURE_WALLETCRYPT` and `FEATURE_COMPRPUBKEY` both of which are incredibly old features. So for such wallets, the behavior of `upgradewallet` will be that the feature is enabled immediately without the wallet needing to be encrypted at that time (note that `FEATURE_WALLETCRYPT` indicates support for encryption, not that the wallet is encrypted) or for a new key to be generated. `CanSupportFeature` would previously indicate whether we could upgrade to `nWalletMaxVersion` not just whether the current wallet version supported a feature. While this property was being used to determine whether we should upgrade to HD and HD chain split, it was also causing a few bugs. Determining whether we should upgrade to HD or HD chain split is resolved by passing into `ScriptPubKeyMan::Upgrade` the version we are upgrading to and checking against that. By removing `nWalletMaxVersion` we also fix a bug where you could upgrade to HD chain split without the pre-split keypool. `nWalletMaxVersion` was also the version that was being reported by `getwalletinfo` which meant that the version reported was not always consistent across restarts as it depended on whether `upgradewallet` was used. Additionally to make the wallet versions consistent with actually supported versions, instead of just setting the wallet version to whatever is given to `upgradewallet`, we normalize the version number to the closest supported version number. For example, if given 150000, we would store and report 139900. Another bug where CHDChain was not being upgraded to the version supporting HD chain split is also fixed by this PR. Lastly several more tests have been added. Some refactoring to the test was made to make these tests easier. These tests check specific upgrading scenarios, such as from non-HD (version 60000) to HD to pre-split keypool. Although not specifically related to `upgradewallet`, `UpgradeKeyMetadata` is now being tested too. Part of the new tests is checking that the wallet files are identical before and after failed upgrades. To facilitate this, a utility function `sha256sum_file` has been added. Another part of the tests is to examine the wallet file itself to ensure that the records in the wallet.dat file have been correctly modified. So a new `bdb.py` module has been added to deserialize the BDB db of the wallet.dat file. This format isn't explicitly documented anywhere, but the code and comments in BDB's source code in file `dbinc/db_page.h` describe it. This module just dumps all of the fields into a dict. ACKs for top commit: MarcoFalke: approach ACK 5f9c0b6360 laanwj: Code review ACK 5f9c0b6360215636cfa62a70d3a70f1feb3977ab jonatack: ACK 5f9c0b6360215636cfa62a70d3a70f1feb3977ab, approach seems fine, code review, only skimmed the test changes but they look well done, rebased on current master, debug built and verified the `wallet_upgradewallet.py` test runs green both before and after running `test/get_previous_releases.py -b v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2` Tree-SHA512: 7c4ebf420850d596a586cb6dd7f2ef39c6477847d12d105fcd362abb07f2a8aa4f7afc5bfd36cbc8b8c72fcdd1de8d2d3f16ad8e8ba736b6f4f31f133fe5feba
2020-11-16Merge #20238: doc: Missing comments for signet parametersMarcoFalke
9c08f3332c12aa30c70aaf390c876cc5c1f90617 doc: Missing comments for signet parameters (kanon) Pull request description: We have such comment in chainparams.cpp. However in Signet the comments are missing. In example... - Mainnet https://github.com/bitcoin/bitcoin/blob/d67883d01e507dd22d1281f4a4860e79d6a46a47/src/chainparams.cpp#L83-L84 - Testnet https://github.com/bitcoin/bitcoin/blob/d67883d01e507dd22d1281f4a4860e79d6a46a47/src/chainparams.cpp#L196-L197 - Regtest https://github.com/bitcoin/bitcoin/blob/d67883d01e507dd22d1281f4a4860e79d6a46a47/src/chainparams.cpp#L392-L393 ACKs for top commit: theStack: ACK 9c08f3332c12aa30c70aaf390c876cc5c1f90617 Tree-SHA512: d4e488cf01e50d6320282b29d776c11e6b3d423f9268226749f738a57a51f456b6bd48334d2d5a43afa782df65ea15525a0af1688003c1be6ef915c05650e147
2020-11-16Merge #20390: CI/Cirrus: Skip merge_base step for non-PRsMarcoFalke
20e491ddcb2617472c15294067768e8ce122499a CI/Cirrus: Skip merge_base step for non-PRs (Luke Dashjr) Pull request description: CIRRUS_BASE_BRANCH is a PR-specific variable and undocumented on non-PR builds. In practice (at the moment), it seems to be HEAD, which in private repositories can be pretty much anything, causing CI to fail if it can't be cleanly merged. By checking CIRRUS_PR first, we can reliably do CI builds of branches outside PRs. ACKs for top commit: MarcoFalke: review ACK 20e491ddcb2617472c15294067768e8ce122499a Tree-SHA512: 9fd8db2e19a3145f7dccfca107631b20df8c94d385f624e2bcef2fa18e38bf3e23c6c68fc8241decedbf1413bf69ca572cff75e1ccf82c09ac50443001ec5ae5
2020-11-16Merge #20033: refactor: minor whitespace fixups, s/const/constexpr/ and ↵MarcoFalke
remove template (followup to #19845) 89836a82eec63f93bbe6c3bd6a52be26e71ab54d style: minor improvements as a followup to #19845 (Vasil Dimov) Pull request description: Address suggestions: https://github.com/bitcoin/bitcoin/pull/19845#discussion_r495486760 https://github.com/bitcoin/bitcoin/pull/19845#discussion_r495488051 https://github.com/bitcoin/bitcoin/pull/19845#discussion_r495730125 ACKs for top commit: jonatack: re-ACK 89836a8 change since previous review is replacing std::runtime_error with std::exception, built/ran unit tests with gcc debian 10.2.0-15, then broke a few v3 net_tests involving `BOOST_CHECK_EXCEPTION`, rebuilt, ran `src/test/test_bitcoin -t net_tests -l all` and checked the error reporting. hebasto: re-ACK 89836a82eec63f93bbe6c3bd6a52be26e71ab54d theStack: ACK 89836a82eec63f93bbe6c3bd6a52be26e71ab54d Tree-SHA512: 36477fdccabe5a8ad91fbabb4655cc363a3a7ca237a98ae6dd4a9fae4a4113762040f864d4ca13a47d081f7d16e5bd487edbfb61ab50a37e4a0424e9bec30b24
2020-11-15Merge #20386: Silence false positive GCC warning in wallet/rpcwallet.cppMarcoFalke
049feabf289ace817a3da62fe84942d0200b8f0b Add missing optional.h include (Kristaps Kaupe) 29c66ace5c777b56cd9e19756a1ab0801d15a5ae Silence false positive GCC warning (Kristaps Kaupe) Pull request description: Resolves #20381. ACKs for top commit: jnewbery: utACK 049feabf289ace817a3da62fe84942d0200b8f0b practicalswift: ACK 049feabf289ace817a3da62fe84942d0200b8f0b: diagnostics signal to noise is increased by getting rid of false positives hebasto: ACK 049feabf289ace817a3da62fe84942d0200b8f0b, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 05d84f51521c3b843ed6bf284a83a91db015ad0cd4fcf8b602275812575c1f6b4899286a89d360fbd3caef184abdfb9d834e119842d8740919892f05a0f9e1f8
2020-11-15Merge #20395: ci: Use the previous build worker image in AppVeyorMarcoFalke
406097c8102d903759dabcbaf94c39831580139b ci: Use the previous build worker image in AppVeyor (Hennadii Stepanov) Pull request description: This is a workaround as the [recent](https://www.appveyor.com/updates/2020/11/14/) Visual Studio 2019 image update breaks our builds. This PR is alternative to #20392 due to its build [failure](https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/36314660). ACKs for top commit: MarcoFalke: review ACK 406097c8102d903759dabcbaf94c39831580139b also seems to pass Tree-SHA512: f9b722d8e67dd7f0745de6da385064630adb27ecbc0a919be47f62217a2bb7a27a6fa00a7536a24bf17500a77160ca3b92b3c8619047171a6f5198b434015221
2020-11-15ci: Use the previous build worker image in AppVeyorHennadii Stepanov
This is a workaround as the recent Visual Studio 2019 image update breaks our builds.
2020-11-14CI/Cirrus: Skip merge_base step for non-PRsLuke Dashjr
CIRRUS_BASE_BRANCH is a PR-specific variable and undocumented on non-PR builds. In practice (at the moment), it seems to be HEAD, which in private repositories can be pretty much anything, causing CI to fail if it can't be cleanly merged. By checking CIRRUS_PR first, we can reliably do CI builds of branches outside PRs.
2020-11-14gui: create wallet: add advanced sectionSjors Provoost
2020-11-14gui: create wallet: name placeholderSjors Provoost
2020-11-14Add missing optional.h includeKristaps Kaupe
2020-11-14Silence false positive GCC warningKristaps Kaupe
2020-11-13Merge #20378: wallet: fix potential division by 0 in WalletLogPrintfJonas Schnelli
440f8d3abe97b96f434dad5216d417a08fc10253 fix potential devision by 0 (Jonas Schnelli) Pull request description: #20344 removed the divide-by-zero sanitizer suppression in `wallet/wallet.cpp` but kept a potential devision by zero in `wallet.cpp`'s fee logging. Detected here https://bitcoinbuilds.org/index.php?job=ffb7d59f-379f-4f27-a273-a5595b8c5f07 ACKs for top commit: practicalswift: ACK 440f8d3abe97b96f434dad5216d417a08fc10253 laanwj: Code review ACK 440f8d3abe97b96f434dad5216d417a08fc10253 hebasto: re-ACK 440f8d3abe97b96f434dad5216d417a08fc10253 Tree-SHA512: 9f7903d1e567497c5f972d39e9629c059151e705dbed0a6b88f7c6650c50ecf820f78e3e0f3e629c661d45a938c5d7659faae7c61e47ca8b3bdb029661bca55a
2020-11-13Merge #19065: tests: Add fuzzing harness for CAddrManMarcoFalke
d04a17a7907c57f7b570e1b9743fd63489bdad68 fuzz: Use ConsumeRandomLengthBitVector(...) in src/test/fuzz/connman and src/test/fuzz/net (practicalswift) e6bb9fde851422808f5d9870782c394f74a1f400 tests: Add fuzzing harness for CAddrMan (practicalswift) Pull request description: Add fuzzing harness for `CAddrMan`. ~~Fill some fuzzing coverage gaps for functions in `addrdb.h`, `merkleblock.h` and `outputtype.h`.~~ See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets). Happy fuzzing :) ACKs for top commit: MarcoFalke: review ACK d04a17a7907c57f7b570e1b9743fd63489bdad68 Tree-SHA512: a6b627e3a0cb51e3a8cb02ad0f19088fc0e965ca34ab110b68d5822d0ea7f473207ae312b49fb217cb6cf2f9f211d00bb69c83bac9f50d79c9ed1e157e85775d
2020-11-13Merge #20379: tests: Remove no longer needed UBSan suppression (float ↵MarcoFalke
divide-by-zero in validation.cpp) 0ccb3addf68067200892963521a92713c4667a63 tests: Remove no longer needed UBSan suppression (float-divide-by-zero in validation.cpp) (practicalswift) Pull request description: Remove no longer needed UBSan suppression. The float divide-by-zero in `validation.cpp` was fixed by instagibbs in ec30a79f1c430cc7fbda37e5d747b0b31b262fa5 (#15283). ACKs for top commit: MarcoFalke: ACK 0ccb3addf68067200892963521a92713c4667a63 Tree-SHA512: 89a4f4b7371fa5725d9f801cee7ebbd17523f66017c9acfa813657dcb8d837f42209eff44ce9e5d48296a630bab9599d75f10024a0c7da7defb228f4eae3392a
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-15 01:51:28 +0000