fa340b8794 refactor: Avoid magic value of all-zeros in assumeutxo base_blockhash (MarcoFalke)
fae33f98e6 Fix assumeutxo crash due to invalid base_blockhash (MarcoFalke)
fa5668bfb3 refactor: Use type-safe assumeutxo hash (MarcoFalke)
0000007709 refactor: Remove unused code (MarcoFalke)
faa921f787 move-only: Add util/hash_type (MarcoFalke)
Pull request description:
Starting with commit d6af06d68a, a block hash of all-zeros is invalid and will lead to a crash of the node. Can be tested by cherry-picking the test changes without the other changes.
Stack trace (copied from https://github.com/bitcoin/bitcoin/pull/21584#discussion_r612673879):
```
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007ffff583c8b1 in __GI_abort () at abort.c:79
#2 0x00007ffff582c42a in __assert_fail_base (fmt=0x7ffff59b3a38 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
assertion=assertion@entry=0x555556c8b450 "!hashBlock.IsNull()", file=file@entry=0x555556c8b464 "txdb.cpp", line=line@entry=89,
function=function@entry=0x555556c8b46d "virtual bool CCoinsViewDB::BatchWrite(CCoinsMap &, const uint256 &)") at assert.c:92
#3 0x00007ffff582c4a2 in __GI___assert_fail (assertion=0x555556c8b450 "!hashBlock.IsNull()", file=0x555556c8b464 "txdb.cpp", line=89,
function=0x555556c8b46d "virtual bool CCoinsViewDB::BatchWrite(CCoinsMap &, const uint256 &)") at assert.c:101
#4 0x000055555636738b in CCoinsViewDB::BatchWrite (this=0x5555577975c0, mapCoins=std::unordered_map with 110 elements = {...}, hashBlock=...) at txdb.cpp:89
#5 0x00005555564a2e80 in CCoinsViewBacked::BatchWrite (this=0x5555577975f8, mapCoins=std::unordered_map with 110 elements = {...}, hashBlock=...) at coins.cpp:30
#6 0x00005555564a43de in CCoinsViewCache::Flush (this=0x55555778eaf0) at coins.cpp:223
#7 0x00005555563fc11d in ChainstateManager::PopulateAndValidateSnapshot (this=0x55555740b038 <g_chainman>, snapshot_chainstate=..., coins_file=..., metadata=...)
at validation.cpp:5422
#8 0x00005555563fab3d in ChainstateManager::ActivateSnapshot (this=0x55555740b038 <g_chainman>, coins_file=..., metadata=..., in_memory=true) at validation.cpp:5299
#9 0x0000555555e8c893 in validation_chainstatemanager_tests::CreateAndActivateUTXOSnapshot<validation_chainstatemanager_tests::chainstatemanager_activate_snapshot::test_method()::$_12>(NodeContext&, boost::filesystem::path, validation_chainstatemanager_tests::chainstatemanager_activate_snapshot::test_method()::$_12) (node=...,
root=..., malleation=...) at test/validation_chainstatemanager_tests.cpp:199
#10 0x0000555555e8877a in validation_chainstatemanager_tests::chainstatemanager_activate_snapshot::test_method (this=0x7fffffffc8d0)
at test/validation_chainstatemanager_tests.cpp:262
ACKs for top commit:
laanwj:
Code review re-ACK fa340b8794
jamesob:
ACK fa340b8794 ([`jamesob/ackr/21584.1.MarcoFalke.fix_assumeutxo_crash_due`](https://github.com/jamesob/bitcoin/tree/ackr/21584.1.MarcoFalke.fix_assumeutxo_crash_due))
Tree-SHA512: c2c4e66c1abfd400ef18a04f22fec1f302f1ff4d27a18050f492f688319deb4ccdd165ff792eee0a1f816e7b69fb64080662b79517ab669e3d26b9eb77802851
bc4538806e build: add *~ to .gitignore (Sjors Provoost)
Pull request description:
The file `configure~` recently started appearing for me on macOS (11.3.1) whenever configure is (re)run.
ACKs for top commit:
hebasto:
ACK bc4538806e, tested on Linux Mint 20.1 with different build scenarios including cross-compiling for Windows and macOS.
Tree-SHA512: 830c7baf392ff6d66250a79c6ed0a98dac3daaace54a6d2e7940b9a72e3bac79ab44bbecd7642c931fde8a446654e2260d6afdecc679a1743fae6ec5eeda79f1
Homebrew autoconf version 2.7.1 introduces configure~ as a build artifact.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
9c891b64ff net: initialize nMessageSize to max uint32_t instead of -1 (eugene)
Pull request description:
nMessageSize is uint32_t and is set to -1. This will warn with `-fsanitize=implicit-integer-sign-change` when V1TransportDeserializer calls into the ctor. This pull initializes nMessageSize to `numeric_limits<uint32_t>::max()` instead and removes the ubsan suppression.
ACKs for top commit:
laanwj:
Code review ACK 9c891b64ff
promag:
Code review ACK 9c891b64ff.
Tree-SHA512: f05173d9553a01d207a5a7f8ff113d9e11354c50b494a67d44d3931c151581599a9da4e28f40edd113f4698ea9115e6092b2a5b7329c841426726772076c1493
faad68fcd4 index: Avoid async shutdown on init error (MarcoFalke)
Pull request description:
An async shutdown during init is confusing when a simple boolean return value can be used for a synchronous shutdown.
This also changes the error message on stderr from:
```
Error: A fatal internal error occurred, see debug.log for details
Error: A fatal internal error occurred, see debug.log for details
```
To:
```
Error: basic block filter index best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)
ACKs for top commit:
laanwj:
Code review ACK faad68fcd4
Tree-SHA512: 92dd895266d6d15a6b1a5c081c9b83f83d5c82e9bfceb3ea0664f48540812239e274c829ff0271c4a0afb6d6a8f67d89c5af20d719982ad62999a41ca0623274
fa74bfc860 fuzz: Run const CScript member functions only once (MarcoFalke)
Pull request description:
Those functions should be O(N) in the input size (or maybe worse, I didn't check), so if the fuzz input dictates to run them N times, the complexity is N^2.
Fix this by calling them only once.
Can be reviewed with: `--ignore-all-space --word-diff-regex=.`
Input: https://github.com/bitcoin/bitcoin/files/6464685/clusterfuzz-testcase-minimized-input.log
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34101
ACKs for top commit:
practicalswift:
cr ACK fa74bfc860: patch looks correct, rationale makes sense and patch touches only `src/test/fuzz/`
Tree-SHA512: d579f7a2103ec154bf482a872142e55a1d9e7673d33a22a4c4230186fdd1b6618846463f4e25941031cc8c4bd1ea8d06cb49ae1bb1ec4af115497f5e5de1e19c
d420e5c1c0 guix-attest: Avoid incomplete sigdirs with ERR traps (Carl Dong)
feda2c8e31 guix: Skip attesting to dist-archive (Carl Dong)
d522d8006b guix: Attest to inputs in inputs.SHA256SUMS (Carl Dong)
f9e2960c01 guix: Construct $OUTDIR in ${DISTSRC}/output (Carl Dong)
022abc85fc guix: Minor quoting fix in libexec/build.sh (Carl Dong)
c83c4fa5b7 guix-attest: Allow skipping GPG signing with NO_SIGN (Carl Dong)
0e1c2e448c guix-attest: Use ascii-armor signatures (Carl Dong)
b5fd89c4c8 guix-attest: Only use cross-platform flags for find+xargs (Carl Dong)
5926432ba6 guix: Add guix-verify script (Carl Dong)
30daf76a97 guix: Add guix-attest script (Carl Dong)
Pull request description:
Adds replacements for `gsign` and `gverify`.
Personally I'm not a big fan of using the word "sign" as it's been used to refer to both codesigning and GPG signing.
ACKs for top commit:
laanwj:
Code review and tested ACK d420e5c1c0
Tree-SHA512: 93d82d201f4596eaea0e3825aa55b013dfb91790e6ccee79893833d37921513d7b4e735f0641103e1e2ea8308abe4cb6218b73160924708802f2e0e3f7f6caf1
792be53d3e refactor: Replace std::bind with lambdas (Hennadii Stepanov)
a508f718f3 refactor: Use appropriate thread constructor (Hennadii Stepanov)
30e4448215 refactor: Make TraceThread a non-template free function (Hennadii Stepanov)
Pull request description:
This PR does not change behavior.
Its goal is to improve readability and maintainability of the code.
ACKs for top commit:
jnewbery:
utACK 792be53d3e
jonatack:
tACK 792be53d3e
MarcoFalke:
cr ACK 792be53d3e
Tree-SHA512: a03142f04f370f6bc02bd3ddfa870819b51740fcd028772241d68c84087f95a2d78207cbd5edb3f7c636fcf2d76192d9c59873f8f0af451d3b05c0cf9cf234df
b95f7f8ac0 build, qt, refactor: Drop sed commands for win32-g++/qmake.conf (Hennadii Stepanov)
Pull request description:
Such possibility is [available](https://codereview.qt-project.org/c/qt/qtbase/+/165348) since Qt 5.8.0.
ACKs for top commit:
fanquake:
ACK b95f7f8ac0
Tree-SHA512: e56a3d208a6bd5d42c722f8b344010fe7d1b6f7a28486613dfcb03f0403a47cee8476e2366eeaac401a19836cd09f782e8741a1e781ab4d78f72c500a30e4929
99993f0664 fuzz: Avoid excessively large min fee rate in tx_pool (MarcoFalke)
Pull request description:
Any fee rate above 1 BTC / kvB is clearly nonsense, so no need to fuzz this.
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34078
ACKs for top commit:
practicalswift:
cr ACK 99993f0664: patch looks correct despite no `fa` prefix in commit hash
Tree-SHA512: bd3651d354b13d889ad1708d2b385ad0479de036de74a237346eefad5dbfb1df76ec02b55ec00487ec598657ef6102f992302b14c4e47f913a9962f81f4157e6
fa95555a49 fuzz: Limit max insertions in timedata fuzz test (MarcoFalke)
Pull request description:
It is debatable whether a size of the median filter other than `200` (the only size used in production) should be fuzzed. For now add a minimal patch to cap the max insertions. Otherwise the complexity is N^2 log(N), where N is the size of the fuzz input.
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34167
ACKs for top commit:
practicalswift:
cr ACK fa95555a49: patch looks correct
Tree-SHA512: be7737e9f4c906053e355641de84dde31fed37ed6be4c5e92e602ca7675dffdaf06b7063b9235ef541b05d3d5fd689c99479317473bb15cb5271b8baabffd0f2
36fb036d25 p2p: allow NetPermissions::ClearFlag() only with PF_ISIMPLICIT (Jon Atack)
4e0d5788ba test: add net permissions noban/download unit test coverage (Jon Atack)
dde69f20a0 p2p, bugfix: use NetPermissions::HasFlag() in CConnman::Bind() (Jon Atack)
Pull request description:
This is a bugfix follow-up to #16248 and #19191 that was noticed in #21506. Both v0.21 and master are affected.
Since #19191, noban is a multi-flag that implies download, so the conditional in `CConnman::Bind()` using a bitwise AND on noban will return the same result for both the noban status and the download status. This means that download peers are incorrectly not being added to local addresses because they are mistakenly seen as noban peers.
The second commit adds unit test coverage to illustrate and test the noban/download relationship and the `NetPermissions` operations involving them.
The final commit adds documentation and disallows calling `NetPermissions::ClearFlag()` with any second param other than `NetPermissionFlags` "implicit" -- per current usage in the codebase -- because `ClearFlag()` should not be called with any second param that is a subflag of a multiflag, e.g. "relay" or "download," as that would leave the result in an invalid state corresponding to none of the existing NetPermissionFlags. Thanks to Vasil Dimov for noticing this.
ACKs for top commit:
theStack:
re-ACK 36fb036d25☕
vasild:
ACK 36fb036d25
hebasto:
ACK 36fb036d25, I have reviewed the code and it looks OK, I agree it can be merged.
kallewoof:
Code review ACK 36fb036d25
Tree-SHA512: 5fbc7ddbf31d06b35bf238f4d77ef311e6b6ef2e1bb9893f32f889c1a0f65774a3710dcb21d94317fe6166df9334a9f2d42630809e7fe8cbd797dd6f6fc49491
fae196147b doc: Clarify that feerates are per virtual size (MarcoFalke)
fa83e95ac6 scripted-diff: Clarify that feerates are per virtual size (MarcoFalke)
Pull request description:
By implementing segwit, it is already clear that all feerates in Bitcoin Core are denoted in (amount/virtual size). Though, there is inconsistency, as some places use kvB, some use kB. Thus, replace all with "kvB".
See also commit 6da3afbaee, which did the replacement for wallet RPCs.
ACKs for top commit:
ryanofsky:
Code review ACK fae196147b. Checked instances where units were being added in the second commit and they all looked right.
Tree-SHA512: ab70d13cde7d55c1ac931bddc2b45aa218fc75ef46cb6ea9e5a30b1d4dbf27889c2b6357299a6c5427912443a46ec3592a4809dae335e03162bd2120a0f7f8ad
34b04eec44 refactor: Add TSA annotations to the WorkQueue class members (Hennadii Stepanov)
Pull request description:
Noted while reviewing #19033, and hoping this will not conflict with it :)
ACKs for top commit:
promag:
Code review ACK 34b04eec44.
Tree-SHA512: 4c15729acd95223263c19bc0dd64b9e7960872b48edee6eee97a5d0c2b99b8838185ac3a2ccd5bee992cb3a12498633427fe9919be5a12da9949fcf69a6275a0
fa4bbd306e refactor: Remove useless extern keyword (MarcoFalke)
Pull request description:
It is redundant, confusing and useless.
https://en.cppreference.com/w/cpp/language/storage_duration#external_linkage
ACKs for top commit:
practicalswift:
cr ACK fa4bbd306e: patch looks correct
Talkless:
utACK fa4bbd306e, built successfully on Debian Sid, looks OK.
jonatack:
Light code review ACK fa4bbd306e
hebasto:
ACK fa4bbd306e, I've verified that all of the remained `extern` keywords specify either (a) a variable with external linkage, or (b) a symbol with "C" language linkage.
promag:
Code review ACK fa4bbd306e.
Tree-SHA512: 1d77d661132defa52ccb2046f7a287deb3669b68835e40ab75a0d9d08fe6efeaf3bea7c0e76c754fd18bfe45972c253a39462014080d014cc5d810498784e3e4
a0f7978674 qt: enable wordWrap for peers-tab detail services (randymcmillan)
Pull request description:
Enable wordWrap for peers-tab detailView Services
ACKs for top commit:
Talkless:
tACK a0f7978674 on same environment as previously.
hebasto:
ACK a0f7978674, tested on Linux Mint 20.1 (Qt 5.12.8):
kristapsk:
re-ACK a0f7978674. Tested under Gentoo Linux with Xfce4 (Qt 5.15.2).
Tree-SHA512: 872e511d2ecfa72fea0fd3284a958b45ee8aee138469ce7f9cd853cd9098b9583917909934b0a5c96f9b81ea1567bcea6a037558829bb79f2a3f413a83df06e6
3bad0b3fad Remove user input from URI error message (unknown)
Pull request description:
Removes the user input from error message to avoid it being used in attacks.
Its not really a vulnerability in Bitcoin Core because involves social engineering, dependency on user environment etc. But this PR improves security and by avoiding abuse of URI error in future.
Example of an attack:
1. User opens a link in firefox:
```
bitcoin:tb1qag2e6yhl52hr53vdxzaxvnjtueupvuftan4yfu%0A%0AWARNING%3A%20DO%20NOT%20CLOSE%20THIS%20WINDOW%20OR%20TURN%20OFF%20YOUR%20PC!%20IF%20YOU%20ABORT%20THIS%20PROCESS%2C%20YOU%20COULD%20DESTROY%20ALL%20OF%20YOU%20DATA!%20PLEASE%20ENSURE%20THAT%20YOUR%20POWER%20CABLE%20IS%20PLUGGED%20IN!%0A%0AYou%20became%20victim%20of%20the%20XYZ%20RANSOMWARE!%0A%0AThe%20hard%20disks%20of%20your%20computer%20have%20been%20encrypted%20with%20a%20military%20grade%20encryption%20algorithm.%20There%20is%20no%20way%20to%20restore%20your%20data%20without%20a%20special%20key.%20You%20can%20purchase%20this%20key%20on%20the%20darknet%20page%20shown%20in%20step%202.%0ATo%20purchase%20your%20key%20and%20restore%20your%20data%2C%20please%20follow%20these%20three%20easy%20steps%3A%0A%0A1.%20Download%20the%20Tor%20browser%20at%20%E2%80%9Chttps%3A%2F%2Fwww.torproject.org%2F%E2%80%9C.%0A2.%20Visit%20one%20of%20the%20following%20pages%20with%20the%20Tor%20Browser%3A%0Ahttp%3A%2F%2Frandomchars.onion%2Fabc123%0A3.%20Send%20BTC%20by%20following%20the%20instructions%20on%20the%20page
```
2. User selects Bitcoin Core to open the link:
3. User is asked to send BTC with some message convincing enough which can be different depending on the victim:
**After this PR** (_No user input mentioned in the error_):
ACKs for top commit:
hebasto:
ACK 3bad0b3fad, tested on Linux Mint 20.1 (Qt 5.12.8).
jarolrod:
tACK 3bad0b3fad
Tree-SHA512: aac2fdfcaa7a9cd6582750c1960682554795640f5aacb78bdae121724e1151da3cbb62b8f8b1e0bc37347afe78b3e9a446277cab8e009d2a1050c0e971f001b3
01d9586ae8 qt: Save/restore RPCConsole geometry only for window (Hennadii Stepanov)
Pull request description:
After using the GUI with `-disablewallet` the "Node window" inherits the geometry of the main window, that could be unexpected for users.
This PR provides independent geometry settings for `RPCConsole` in both modes:
- window sizes and `QSplitter` sizes when `-disablewallet=0`
- only `QSplitter` sizes when `-disablewallet=1`
ACKs for top commit:
Talkless:
tACK 01d9586ae8, tested on Debian Sid with Qt 5.15.2. I've managed to reproduce issue using https://github.com/bitcoin-core/gui/pull/194#issuecomment-782822663 instructions, and I see that this PR does detach main window and information window sizes. Built with `--enable-wallet` and `--disable-wallet`.
jarolrod:
ACK 01d9586ae8, tested on macOS 11.2 Qt 5.15.2
promag:
Code review ACK 01d9586ae8.
Tree-SHA512: 9934cf04d4d5070dfc4671ea950e225cda9988858227e5481dad1baafa14af477bdbf4f91307ca687fde0cad6e4e605a3a99377e70d67eb115a19955ce2516f5
bd7f27d16d refactor: feature_csv_activation.py: move tx helper functions to methods (Sebastian Falbesoner)
2eca46b0aa test: use MiniWallet for feature_csv_activation.py (Sebastian Falbesoner)
Pull request description:
This PR enables one more of the non-wallet functional tests (feature_csv_activation.py) to be run even with the Bitcoin Core wallet disabled by using the new MiniWallet instead, as proposed in #20078.
Short reviewers guideline:
- Since we exclusively work with anyone-can-spend outputs here (raw scriptPubKey = OP_TRUE), signing is not needed anymore. The function `sign_transaction` and its calls are removed, after changing a tx (e.g. its scriptSig or nVersion) a simple `.rehash()` call is sufficient. Also, generating an address `self.nodeaddress` (and with that, passing it to the the various test tx creation/sending helper methods) is not needed anymore and removed.
- The test repeatedly uses the same input for creating different txs (e.g. with different txversions 1 and 2). To let `MiniWallet` create a tx with a specific input, we have to call `.get_utxo()` before which also marks the UTXO as spent. The method is changed to also support keeping the UTXO in its internal list (`mark_as_spent=False`). With the behaviour on master, the second call to `.get_utxo()` with the same input would fail.
- To keep the diff in the first commit short, the `miniwallet` is set as a global variable, to avoid passing it on every tx creation/spending helper. The global is eliminated in the second (refactoring) commit, where all the helpers are moved to the test class as methods. By that, we can use `self.nodes[0]` directly in the helpers and don't have to pass it again and again. I think there could still be a lot of improvements/refactoring done in the test, but that should hopefully serve as a good basis.
ACKs for top commit:
laanwj:
Code review ACK bd7f27d16d
MarcoFalke:
review ACK bd7f27d16d🐕
Tree-SHA512: 24fb6a0f7702bae40d5271d197119827067d4b597e954d182e4c1aa5d0fa870368eb3ffed469b26713fa8ff8eb3ecc06abc80b2449cd68156d5559e7ae8a2b11
11d6459b6e rpc: include_unsafe option for fundrawtransaction (t-bast)
Pull request description:
Allow RPC users to opt-in to unsafe inputs when funding a raw transaction.
Applications that need to manage a complex RBF flow (such as lightning nodes using anchor outputs) are very limited if they can only use safe inputs.
I also added this option to `send` and `walletcreatefundedpsbt` who internally delegate to `fundrawtransaction`.
Fixes#21299
ACKs for top commit:
laanwj:
Code review ACK 11d6459b6e
Tree-SHA512: 5e542a4febcfd6f41cf784678ff02ec9282eae2082c274983f72c5ea87b7ebbe1bd5fdc6a020d7a9d5996157754eb4966b8aeb6c1ceebf0b1519f735579b8bac
3adde72bc9 qt: Do not use QObject::tr plural syntax for numbers with a unit symbol (Hennadii Stepanov)
Pull request description:
Working on translation, I found this is useless and unnecessarily burdensome for translators. I guess, this statement is correct internationally wide :)
ACKs for top commit:
jarolrod:
ACK 3adde72bc9
promag:
Code review ACK 3adde72bc9. Agree with OP, looks reasonable to me.
Tree-SHA512: bde65c122ca0feb7771d932cce63fd1aef1e7a9dda0188d19c577d57b279172204ac1bfcb6106a78b2c4d55d628e6dc0967051e064ec40d3c5aeafd4a48f0589
facfc0f65d fuzz: Remove strprintf test cases that are known to fail (MarcoFalke)
Pull request description:
They are still waiting to be fixed (see https://github.com/c42f/tinyformat/issues/70 ), so no need for us to carry them around in our source code. They can be added back once upstream is fixed.
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34082
ACKs for top commit:
laanwj:
Code review ACK facfc0f65d
Tree-SHA512: d9d3d35555b6d58740a041ae45797ca85149f60990e2ed632c5dadf363e1d2362d2447681d7ceaa1fbffcd6e7bc8da5bc15d3923b68829a86c25b364a599afc8
71c824ed6c cleaned up and added missing "include" statements for pubkey.cpp and pubkey.h (William Bright)
Pull request description:
#### Problem:
Many symbols in the files were undefined and causing issues when I was working on building independent sections of the codebase. The hidden imports from the "secp256k1" library was a particular pain point.
The other standard and missing includes are following best practices and will help with refactoring, build process and others.
#### Changes:
Clean up and declared imports/include for `pubkey.cpp` and `pubkey.h`
ACKs for top commit:
jnewbery:
utACK 71c824ed6c
laanwj:
Code review ACK 71c824ed6c
Tree-SHA512: bce605cfde24d8e3be82a596cabab7a8577fec0aef7c5e6f7a56603357046d8e8dea11ac8e3dbe79600550291be7784e35c7a55ebf40b46525b8949e4bedae96
d66f283ac0 scripted-diff: Replace three dots with ellipsis in the UI strings (Hennadii Stepanov)
Pull request description:
This PR is split from #21463.
The change was suggested on [Transifex.com](https://www.transifex.com/bitcoin/bitcoin/), and it does not touch `LogPrint` and `LogPrintf` calls.
The only comment on #21463 [was](9030e4b5a6 (r597220100)):
> Mind that these messages also end up in the log. In principle the log is already UTF-8 (as are all strings and text in bitcoind). But, just noting, that it might make browsing the log a less pleasant experience on systems with misconfigured locale like some BSDs by default.
ACKs for top commit:
laanwj:
ACK d66f283ac0
Tree-SHA512: 5ab1cb3160f3f996f1ad7d7486662da3eb7f06a857f4a1874963ce10caed5b86b0ad6151b1b9ebeb2b8aa5f0c85efad3b768ea9cafe5db86f78f88912b756d1e
f52fafc935 build: Drop pointless sed commands (Hennadii Stepanov)
Pull request description:
Since moving to Autotools build system (35b8af9226, #2943, 2013-09), tag strings created by Qt specialized compilers ([uic](https://doc.qt.io/qt-5/uic.html), [moc](https://doc.qt.io/qt-5/moc.html), [rcc](https://doc.qt.io/qt-5/rcc.html)) were being removed.
A bit later (70c71c50ce, #4241, 2014-06) this rule was dropped for the uic, and since then all of the generated `ui_*.h` files contain the following string:
```
** Created by: Qt User Interface Compiler version 5.12.8
```
Such strings do not contain any timestamps, and cannot cause any non-determinism. The removing of them seems pointless.
Diffs for some files:
```diff
--- master/intro.moc
+++ pr/intro.moc
@@ -1,6 +1,7 @@
/****************************************************************************
** Meta object code from reading C++ file 'intro.cpp'
**
+** Created by: The Qt Meta Object Compiler version 67 (Qt 5.12.8)
**
** WARNING! All changes made in this file will be lost!
*****************************************************************************/
```
```diff
--- master/moc_addressbookpage.cpp
+++ pr/moc_addressbookpage.cpp
@@ -1,6 +1,7 @@
/****************************************************************************
** Meta object code from reading C++ file 'addressbookpage.h'
**
+** Created by: The Qt Meta Object Compiler version 67 (Qt 5.12.8)
**
** WARNING! All changes made in this file will be lost!
*****************************************************************************/
```
```diff
--- master/qrc_bitcoin.cpp
+++ pr/qrc_bitcoin.cpp
@@ -1,6 +1,7 @@
/****************************************************************************
** Resource object code
**
+** Created by: The Resource Compiler for Qt version 5.12.8
**
** WARNING! All changes made in this file will be lost!
*****************************************************************************/
```
ACKs for top commit:
laanwj:
ACK f52fafc935
Tree-SHA512: 31f5c19b37645b4914f17d8c234b7ae8781a0499c4b250ffef07d70b7552954fb682f58a75d76162f98ab5e1667288b3a041df2705573fb00523e87b9c1fd47f
847288df07 test: fee rate values that cannot be represented as sat/vB (Jon Atack)
06a90fa038 rpc: for sat/vB fee rates, limit ParseFixedPoint decimals to 3 (Jon Atack)
0742c7840f rpc: enable passing decimals to AmountFromValue, add doxygen (Jon Atack)
8ce3ef57a3 test: ParseFixedPoint with 3 decimals for sat/vB fee rates (Jon Atack)
b503327597 test: type error and out of range fee rates where missing (Jon Atack)
c5fd4344f7 test: explicit fee rates with invalid amounts (Jon Atack)
ea6f76b66e test: improve zero-value explicit fee rate coverage (Jon Atack)
Pull request description:
- Improve/close gaps in existing test coverage before making the change
- Enable passing `decimals` to `ParseFixedPoint()` when calling `AmountFromValue()`
- Limit explicit fee rates in sat/vB passed in by users to 3 decimals, and raise otherwise
- Add regression test coverage
Closes#20534.
ACKs for top commit:
MarcoFalke:
review ACK 847288df07🔷
Tree-SHA512: c539d07ae9b21c0d6c8ea460beb9c8dad5559445518aace560abc3c05c588907bae189b6fd7602b3b397de4a42356136c3ec6f960d3dcf2d5d16377aef4ab5a2
7031721f2c rpc/listaddressgroupings: redefine inner-most array as ARR_FIXED (Karl-Johan Alm)
8500f7bf54 rpc/createrawtransaction: redefine addresses as OBJ_USER_KEYS (Karl-Johan Alm)
d9e2183c50 rpc: include OBJ_USER_KEY in RPCArg constructor checks (Karl-Johan Alm)
Pull request description:
This PR adjusts the two issues I encountered while developing a tool that converts RPCHelpMan objects into bindings for other language(s).
The first is in createrawtransaction, where the address part, e.g. bc1qabc in
> createrawtransaction '[]' '[{"bc1qabc": 1.0}]'
is declared as a `Type::OBJ`, when in reality it should be a `Type::OBJ_USER_KEYS`, defined as such:
5925f1e652/src/rpc/util.h (L126)
(coincidentally, this is the first and only (afaict) usage of this `RPCArg::Type`).
The second is in the `listaddressgroupings` RPC, which returns an array of arrays of arrays, where the innermost one is a tuple-thingie with an optional 3rd item; this is an `ARR_FIXED`, not an `ARR`.
ACKs for top commit:
MarcoFalke:
ACK 7031721f2c🐀
Tree-SHA512: 769377416c6226d1738a956fb685498e009f9e7eb2d45bc679b81c5364b9520fdbcb49392c937ab45598aa0d33589e8e6a59ccc101cf8d8e7dfdafd58d4eefd0
fa2204f6ad streams: Accept URef obj for VectorReader unserialize (MarcoFalke)
Pull request description:
Missed in commit 172f5fa738. An URef may collapse into an LRef or RRef depending on context. There is no reason to forbid RRef in `VectorReader::operator>>`, so add it for consistency.
ACKs for top commit:
ryanofsky:
Code review ACK fa2204f6ad, just expanded test since last review
Tree-SHA512: 09ff4e8a918e15b08cebd8c125d37e78bfb3a635c38546fc8454a97a882b2c81c55ef552243617e78744799d31127e6fbf78c4e319c030480b370aab6f38b645
fa1aa6c571 fuzz: Limit ParseISO8601DateTime fuzzing to 32-bit (MarcoFalke)
Pull request description:
2038 is more than 10 years in the future, so no need for us to waste time fuzzing a 3rd party lib that will be EOL by then.
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34092
ACKs for top commit:
theStack:
Concept and code review ACK fa1aa6c571
Tree-SHA512: fdd2fbc7b5c7ce33ad23b2e5431bb97eaf6ae8c2d2a55990a3ab73be79282c584b704dcd1471ba288de75283732970c70c9a03ddad059b97b66ba8b3de39effe
This allows to get rid of the global miniwallet variable and to specify
the used node self.nodes[0] at only one place, instead of passing it to
every tx creation/send method again and again.
Can be reviewed with --ignore-all-space --color-moved=dimmed-zebra
