aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2024-09-30Merge bitcoin/bitcoin#30968: init: Remove retry for loopAva Chow
e9d60af9889c12b4d427adefa53fd234e417f8f6 refactor: Replace init retry for loop with if statement (TheCharlatan) c1d8870ea4155c2766d30d38fc5b1afc63dcd364 refactor: Move most of init retry for loop to a function (TheCharlatan) 781c01f58066d375c14b1a717160f51c2f2ebe20 init: Check mempool arguments in AppInitParameterInteractions (TheCharlatan) Pull request description: The for loop around the chain loading logic in `init.cpp` allows users of the GUI to retry once on failure with reindexing without having to manually set the reindex flag on startup. However this current mechanism has problems: * It is badly documented and has led to confusion among developers and bugs making it into master. Examples: * https://github.com/bitcoin/bitcoin/pull/28830/files#r1598392660 * https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2120741121 * It can only ever iterate once, making the choice of a for loop questionable. * With its large scope it is easy for re-entry bugs to sneak in. Example: * https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1601589963 Attempt to fix this by moving the bulk of the logic into a separate function and replacing the for loop with a simpler `if` statement. The diff's in this pull request are best reviewed with `--color-moved-ws=ignore-all-space --color-moved=dimmed-zebra`. The error behaviour can be tested by either manually making `LoadChainstate` return a failure, or deleting some of the block index database files. ACKs for top commit: maflcko: review ACK e9d60af9889c12b4d427adefa53fd234e417f8f6 🚸 josibake: crACK https://github.com/bitcoin/bitcoin/commit/e9d60af9889c12b4d427adefa53fd234e417f8f6 achow101: ACK e9d60af9889c12b4d427adefa53fd234e417f8f6 ryanofsky: Code review ACK e9d60af9889c12b4d427adefa53fd234e417f8f6. Nice change to make AppInitMain shorter and more understandable. Tree-SHA512: 5e5c0a5fd1b32225346450f8482f0ae8792e1557cdab1518112c1a3ec3a4400b64f5796692245cc5bf2f9010bb97b3a9558f07626a285ccd6ae525dd671ead13
2024-09-30Merge bitcoin/bitcoin#30043: net: Replace libnatpmp with built-in PCP+NATPMP ↵Ava Chow
implementation 5c7cacf649a6b474b876a7d219c7dc683a25e33d ci: Remove natpmp build option and libnatpmp dependency (laanwj) 7e7ec984da50f45491b994aaab180e7735ad1d8f doc: Remove mention of natpmp build options (laanwj) 061c3e32a26c6c04bf734d62627403758d7e51d9 depends: Drop natpmp and associated option from depends (laanwj) 20a18bf6aa38e87f72e2645482d00d0c77a344f5 build: Drop libnatpmp from build system (laanwj) 7b04709862f48e9020c7bef79cb31dd794cf91d0 qt: Changes for built-in PCP+NAT-PMP (laanwj) 52f8ef66c61b82457a161f3b90cc87f57d1dda80 net: Replace libnatpmp with built-in NATPMP+PCP implementation in mapport (laanwj) 97c97177cdb2f596aa7d4a65c4bde87de50a96f2 net: Add PCP and NATPMP implementation (laanwj) d72df63d16941576b3523cfeaa49985cf3cd4d42 net: Use GetLocalAddresses in Discover (laanwj) e02030432b77abbf27bb4f67d879d3ad6d6366e6 net: Add netif utility (laanwj) 754e4254388ec8ac1be6cf807bf300cd43fd3da5 crypto: Add missing WriteBE16 function (laanwj) Pull request description: Continues #30005. Closes #17012.. This PR adds PCP (Port Control Protocol) from [RFC6887](https://datatracker.ietf.org/doc/html/rfc6887). This adds, in addition to the existing IPv4 port mapping (which now uses PCP, with fallback to NAT-PMP), support for IPv6 pinholing-that is, opening a port on the firewall to make it reachable. PCP, like NAT-PMP is a simple UDP-based protocol, and the implementation is self-contained, so this gets rid of lthe libnatpnp dependency without adding a new one. It should otherwise be a drop-in replacement. NAT-PMP fallback is implemented so this will not make router support worse. For now it is disabled by default, though in the future (not in this PR) we could consider enable it by default to increase the number of connectable nodes without adding significant attack surface. To test: ```bash bitcoind -regtest -natpmp=1 -debug=net ``` (most of the changes in this PR are, ironically, removing the libnatpmp dependency and associated build system and build docs) ## TODO - [x] Default gateway discovery on Linux / FreeBSD - [x] Default gateway discovery on Windows - [x] Default gateway discovery on MacOS - [x] Either solve FreeBSD compile issue (probably upstream issue) or remove FreeBSD support ## Things to consider for follow-up PRs - https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1658764974 avoid unreachable nets (not given to -onlynet=) - https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1658949236 could announce an addr:port where we do not listen (no -bind) - https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1684368824 could announce the wrong port because it uses GetListenPort() - https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1679709347 if we requested one port but another was assigned, then which one to use in the renewal? - https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1772017020 Use `GetAdapterAddresses` to discover local addresses for Windows ACKs for top commit: Sjors: ACK 5c7cacf649a6b474b876a7d219c7dc683a25e33d achow101: ACK 5c7cacf649a6b474b876a7d219c7dc683a25e33d vasild: ACK 5c7cacf649a6b474b876a7d219c7dc683a25e33d Tree-SHA512: e35b69e56d5f5449a3d48a799f9b7b65107c65eeb3e245c2c1e9d42221e469ca5ead90afae423160601cd664dd553a51c859e04f4492f335b064aae3bf23e3bc
2024-09-30build: Drop libnatpmp from build systemlaanwj
2024-09-30qt: Changes for built-in PCP+NAT-PMPlaanwj
Change option help, and remove conditionals.
2024-09-30net: Replace libnatpmp with built-in NATPMP+PCP implementation in mapportlaanwj
2024-09-30net: Add PCP and NATPMP implementationlaanwj
Add a RFC 6886 NATPMP and RFC 6887 Port Control Protocol (PCP) implementation, to replace libnatpmp.
2024-09-27Merge bitcoin/bitcoin#30921: test: generalize HasReason and use it in ↵merge-script
FailFmtWithError 6c3c619b35cc03e883f9d2b3326f906aedde9ba7 test: generalize HasReason and use it in FailFmtWithError (LÅ‘rinc) Pull request description: Standardized boost exception checking in recent tests introduced in https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756493521 by extending `HasReason` to accept `const char*` through `string_view` in `operator()`. Note that `HasReason` only checks partial matches - but since we're specifying the whole error string, it doesn't affect us in this case. ACKs for top commit: maflcko: review ACK 6c3c619b35cc03e883f9d2b3326f906aedde9ba7 hodlinator: ACK 6c3c619b35cc03e883f9d2b3326f906aedde9ba7 Tree-SHA512: 740fb18b8fea78e4eb9740ceb0fe75d37246c28cfa2638b9d093e9514dd6d7926cc5be9ec57f8027cca3aa9d616e8c54322d2401cfa67fd25282f7816e63532d
2024-09-26doc: fix loadtxoutset examplefanquake
The current order is incorrect: ```bash ./build/src/bitcoin-cli loadtxoutset -rpcclienttimeout=0 utxo-840000.dat error code: -1 error message: loadtxoutset "path" ```
2024-09-25Merge bitcoin/bitcoin#30510: multiprocess: Add IPC wrapper for Mining interfaceAva Chow
1a332817665f77f55090fa166930fec0e5500727 doc: multiprocess documentation improvements (Ryan Ofsky) d043950ba245cdcd09dc389a71d43b0a58e41a8b multiprocess: Add serialization code for BlockValidationState (Ryan Ofsky) 33c2eee285e35db50b33efb6e782ff002f9d18ec multiprocess: Add IPC wrapper for Mining interface (Ryan Ofsky) 06882f84017f6b569b46a644f39b6d3c120ec6cf multiprocess: Add serialization code for vector<char> (Russell Yanofsky) 095286f790acda4a32f04c77aa86106007e2a0d8 multiprocess: Add serialization code for CTransaction (Russell Yanofsky) 69dfeb18761cfd933b8a9a6e69ce9d3c516ba949 multiprocess: update common-types.h to use C++20 concepts (Ryan Ofsky) 206c6e78eec7c71d317666a1af07acf357ba001e build: Make bitcoin_ipc_test depend on bitcoin_ipc (Ryan Ofsky) 070e6a32d5ff4be2f4f1f6a8200fba2f61df16e3 depends: Update libmultiprocess library for cmake headers target (Ryan Ofsky) Pull request description: Add Cap'n Proto wrapper for the Mining interface introduced in #30200, and its associated types. This PR combined with #30509 will allow a separate mining process, like the one being implemented in https://github.com/Sjors/bitcoin/pull/48, to connect to the node over IPC, and create, manage, and submit block templates. (#30437 shows another simpler demo of a process using the Mining interface.) --- This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722). ACKs for top commit: achow101: ACK 1a332817665f77f55090fa166930fec0e5500727 TheCharlatan: ACK 1a332817665f77f55090fa166930fec0e5500727 itornaza: ACK 1a332817665f77f55090fa166930fec0e5500727 Tree-SHA512: 0791078dd6885dbd81e3d14c75fffff3da8d1277873af379ea6f9633e910c11485bb324e4cde3d936d50d343b16a10b0e8fc1e0fc6d7bdca7f522211da50c01e
2024-09-25refactor: Replace init retry for loop with if statementTheCharlatan
The for loop has been a long standing source of confusion and bugs, both because its purpose is not clearly documented and because the body of the for loop contains a lot of logic. Co-Authored-By: Ryan Ofsky <ryan@ofsky.org>
2024-09-25refactor: Move most of init retry for loop to a functionTheCharlatan
This makes it clearer which state is being mutated by the function and facilitates getting rid of the for loop in the following commit. Move creation of the required options into the function too, such that the function takes fewer arguments and is more self-contained. Co-Authored-By: Ryan Ofsky <ryan@ofsky.org>
2024-09-25init: Check mempool arguments in AppInitParameterInteractionsTheCharlatan
This makes the handling more consistent and reports errors sooner.
2024-09-24validation: Disable CheckForkWarningConditions for background chainstateMartin Zumsande
The comparison of m_best_invalid with the tip of the respective chainstate makes no sense for the background chainstate, and can lead to incorrect error messages.
2024-09-24Merge bitcoin/bitcoin#30952: test: Use shell builtins in run_command test casemerge-script
7bd3ee62f6d6f59ca599e85f81776d282dee1539 test: Use shell builtins in run_command test case (Ava Chow) Pull request description: Uses the [suggested command](https://github.com/bitcoin/bitcoin/issues/30938#issuecomment-2363906135) Fixes #30938 ACKs for top commit: maflcko: review ACK 7bd3ee62f6d6f59ca599e85f81776d282dee1539 hebasto: ACK 7bd3ee62f6d6f59ca599e85f81776d282dee1539. Tree-SHA512: 683b15cafaf0103eeadf872ea6ce9a7d884b2605d3dcf4e66b0173cdb149c24965e7c5fa62aaddf2ac55df3f449aeb787176992c96cfee5d0b86621259e1dfe9
2024-09-23Merge bitcoin/bitcoin#30678: wallet: Write best block to disk before backupAva Chow
f20fe33e94c6752e5d2ed92511c0bf51a10716ee test: Add basic balance coverage to wallet_assumeutxo.py (Fabian Jahr) 037b101e808ccf9e717751619e04f6e87d614efd test: Add coverage for best block locator write in wallet_backup (Fabian Jahr) 31c0df038909e40fe9618a4595254907ed1de907 wallet: migration, write best locator before unloading wallet (furszy) 7e3dbe4180cbeb65e59b53d9fa98509e9189549d wallet: Write best block to disk before backup (Fabian Jahr) Pull request description: I discovered that we don't write the best block to disk when trying to explain the behavior described here: https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1719951882 In the context of that test, the behavior is confusing and I think it also shows that one of the already existing tests in `wallet_assumeutxo.py` doesn't actually test what it says. It only fails because the best block isn't written and actually, the height of the backup that is loaded is at the snapshot height during backup. So it really shouldn't fail since it's past the background sync blocks already. I'm not sure if this is super relevant in practice though so I am first looking for concept ACKs on the `BackupWallet` code change. Either way, I think this behavior should be documented better if it is left as is and the test should be changed. ACKs for top commit: achow101: ACK f20fe33e94c6752e5d2ed92511c0bf51a10716ee furszy: ACK f20fe33 Tree-SHA512: bb384a940df5c942fffe2eb06314ade4fc5d9b924012bfef3b1c456c4182a30825d1e137d8ae561d93d3a8a2f4d1c1ffe568132d20fa7d04844f1e289ab4a28b
2024-09-23multiprocess: Add serialization code for BlockValidationStateRyan Ofsky
Co-authored-by: TheCharlatan <seb.kung@gmail.com>
2024-09-23multiprocess: Add IPC wrapper for Mining interfaceRyan Ofsky
2024-09-23multiprocess: Add serialization code for vector<char>Russell Yanofsky
2024-09-23multiprocess: Add serialization code for CTransactionRussell Yanofsky
Add support for passing CTransaction and CTransactionRef types to IPC functions. These types can't be passed currently because IPC serialization code currently only supports deserializing types that have an Unserialize() method, which CTransaction does not, because it is supposed to represent immutable transactions. Work around this by adding a CustomReadField overload that will call CTransaction's deserialize_type constructor. These types also can't be passed currently because serializing transactions requires TransactionSerParams to be set. Fix this by setting TX_WITH_WITNESS as default serialization parameters for IPC code.
2024-09-23multiprocess: update common-types.h to use C++20 conceptsRyan Ofsky
Idea came from review comment by ion- https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1757372497
2024-09-23build: Make bitcoin_ipc_test depend on bitcoin_ipcRyan Ofsky
This change is needed to allow generated capnp code in src/ipc/capnp/ to be used in unit tests for better test coverage in upcoming commits.
2024-09-23Merge bitcoin/bitcoin#30409: Introduce waitTipChanged() mining interface, ↵Ava Chow
replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block 7942951e3fcc58f7db0059546d03be9ea243f1db Remove unused g_best_block (Ryan Ofsky) e3a560ca68d79b056a105a65ed0c174a9631aba9 rpc: use waitTipChanged for longpoll (Ryan Ofsky) 460687a09c2af336fce853d9ffb790d01429eec6 Remove unused CRPCSignals (Sjors Provoost) dca923150e3ac10a57c23a7e29e76516d32ec10d Replace RPCNotifyBlockChange with waitTipChanged() (Sjors Provoost) 2a40ee1121903847cdd3f6c5b4796e4d5471b2df rpc: check for negative timeout arg in waitfor* (Sjors Provoost) de7c855b3af99fe6b62279c5c2d08888a5437c4a rpc: recommend -rpcclienttimeout=0 for waitfor* (Sjors Provoost) 77ec072925a6d558b3c6b075becbed44727c5989 rpc: fix waitfornewblock description (Sjors Provoost) 285fe9fb51c808a9edd91b05bd3134fc18de0fb6 rpc: add test for waitforblock and waitfornewblock (Sjors Provoost) b94b27cf05c709674117e308e441a8d1efddcd0a Add waitTipChanged to Mining interface (Sjors Provoost) 7eccdaf16081d6f624c4dc21df75b0474e049d2b node: Track last block that received a blockTip notification (Sjors Provoost) ebb8215f23644f901c46fd4977b7d4b08fae5104 Rename getTipHash() to getTip() and return BlockRef (Sjors Provoost) 89a8f74bbbb900abfb3d8e946eea18ad7b1513ad refactor: rename BlockKey to BlockRef (Sjors Provoost) Pull request description: This continues the work in #30200 so that a future Stratum v2 Template Provider (see #29432) can avoid accessing node internals. It needs to know when a new block arrives in order to push new templates to connected clients. `waitTipChanged()` uses a new kernel notification `notifications().m_tip_block_mutex`, which this PR also introduces (a previous version used `g_best_block`). In order to ensure the new method works as intended, the `waitfornewblock`, `waitforblock` and `waitforblockheight` RPC methods are refactored to use it. This allows removing `RPCNotifyBlockChange`. There's a commit to add (direct) tests for the methods that are about to be refactored: - `waitfornewblock` was already implicitly tested by `feature_shutdown.py`. - `waitforblockheight` by `feature_coinstatsindex.py` and `example_test.py` This PR renames `getTipHash()` to `getTip()` and returns a `BlockRef` (renamed from `BlockKey`) so that callers can use either the height or hash. The later commits make trivial improvements to the `waitfor*` RPC calls (not needed for this PR). The `waitTipChanged()` method could probably also be used for the longpoll functionality in `getblocktemplate`, but I'm a bit reluctant to touch that. `RPCServer::OnStarted` no longer does anything and `RPCServer::OnStopped` merely prints a log statement. They were added in #5711 as a refactor. This PR drops them entirely. Finally `g_best_block` is also dropped. ACKs for top commit: achow101: ACK 7942951e3fcc58f7db0059546d03be9ea243f1db ryanofsky: Code review ACK 7942951e3fcc58f7db0059546d03be9ea243f1db. Just rebased since last review TheCharlatan: Re-ACK 7942951e3fcc58f7db0059546d03be9ea243f1db Tree-SHA512: a5559446b4000c95e07aad33284b7ee2e57aafd87e1ae778b3825d59689566d047a8047e47a10f76e6e341e7dc72fd265a65afbc0a9c011d17c4cafd55031837
2024-09-23test: Use shell builtins in run_command test caseAva Chow
2024-09-21net: Use GetLocalAddresses in Discoverlaanwj
This has the same code, it's unnecessary to duplicate it.
2024-09-21net: Add netif utilitylaanwj
This adds an utility header with two functions that will be needed for PCP, `QueryDefaultGateway` and `GetLocalAddresses`. Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
2024-09-21crypto: Add missing WriteBE16 functionlaanwj
Also add fuzz test, mimicing the WriteLE16 one.
2024-09-20Merge bitcoin/bitcoin#30765: refactor: Allow `CScript`'s `operator<<` to ↵Ava Chow
accept spans, not just vectors 5e190cd11f6ed8b7ab4db0f01192de63deaf6fd7 Replace CScript _hex_v_u8 appends with _hex (LÅ‘rinc) cac846c2fbf6fc69bfc288fd387aa3f68d84d584 Allow CScript's operator<< to accept spans, not just vectors (LÅ‘rinc) c78d8ff4cb83506413bb73833fc5c04885d0ece8 prevector: avoid GCC bogus warnings in insert method (LÅ‘rinc) Pull request description: Split out of https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1722326803. Replace `_hex_v_u8` for `CScript` appends to `_hex`, to skip vector conversion before serializing to the `prevector` in `CScript`. To enable both `unsigned char` and `std::byte` values, I've extracted the existing serialization to append the size & data in separate private methods to clarify that it does more than just a simple data insertion. There were also discussion on eliminating the operators here completely to obviate when we're serializing fixed-size collections as raw bytes, and when we're prefixing them with their size - should also be done in a separate PR. ACKs for top commit: achow101: ACK 5e190cd11f6ed8b7ab4db0f01192de63deaf6fd7 ryanofsky: Code review ACK 5e190cd11f6ed8b7ab4db0f01192de63deaf6fd7. Looks good! hodlinator: re-ACK 5e190cd11f6ed8b7ab4db0f01192de63deaf6fd7 Tree-SHA512: 27a646629e017b2a05416d5eb964dda8b25b900d466221eff7bfa1339ded443e1c5c4cf8ff20cb3bba915a2603787a9fa6f6ec12bc0b9415d9eb07b57289192b
2024-09-20Merge bitcoin/bitcoin#30918: fuzz: Add check in `p2p_headers_presync` that ↵Ava Chow
chain work never exceeds minimum work 284bd17309ab3b124d9dcddfec62f5506383343b add check that chainwork doesn't exceed minimum work (marcofleon) 9aa5d1c3fcd10ecb94310ad515a8569bc2d418f8 add clarification in comment (marcofleon) Pull request description: A followup to https://github.com/bitcoin/bitcoin/pull/30661 The added assertion just makes sure that the fuzz test is working as intended. If we're sure that the total work of the test chain is never more than minimum chain work, then we can be sure that the later assertion failure would actually mean that a bug in the headers presync logic was found. This PR also addresses: https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1746614616 https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1764943665 https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1764961991 ACKs for top commit: instagibbs: reACK 284bd17309ab3b124d9dcddfec62f5506383343b maflcko: review ACK 284bd17309ab3b124d9dcddfec62f5506383343b achow101: ACK 284bd17309ab3b124d9dcddfec62f5506383343b Tree-SHA512: 76a9dffea4b6e13499c636d6ad26af06135319d25117c0eb40cf8dfcfdca6a4549c9b4d2ba835192ca355e0f8d476227aeabf8bdb68770def72a9fb521533fe5
2024-09-20Merge bitcoin/bitcoin#30826: fuzz: reduce number of iterations in ↵Ava Chow
`crypto_aeadchacha20poly1305` target f482d0e366a84008129913b442f0c955de79ac93 fuzz: reduce number of iterations in `crypto_aeadchacha20poly1305` target (brunoerg) Pull request description: By reducing the number of iterations we improve the performance of this target and may increase coverage. Running with `-runs=100000` from qa-assets I noticed a significant performance improvement and an increase on cov: master: ``` #100000 DONE cov: 567 ft: 4078 corp: 124/33Kb lim: 4096 exec/s: 793 rss: 499Mb ``` PR: ``` #100000 DONE cov: 568 ft: 3833 corp: 113/15188b lim: 1746 exec/s: 1250 rss: 544Mb ``` ACKs for top commit: achow101: ACK f482d0e366a84008129913b442f0c955de79ac93 marcofleon: Tested ACK f482d0e366a84008129913b442f0c955de79ac93. Saw the same slight increase in coverage. Executed 100,000 runs several times and total time went from 30-35 sec to 20-25 sec. stratospher: ACK f482d0e. saw similar coverage stats Tree-SHA512: 1a96dbc22a0aed396b7f8cc9b13534b7f20a461f64f167c69c650529d535e360499f1a501abc1f957f7541ee1860b36a5580aa488a1edbfa0270c9ed83ef741d
2024-09-20Merge bitcoin/bitcoin#30794: interpreter: use int32_t instead of int type ↵Ava Chow
for risczero compile bc52cda1f3c007bdf1ed00aa3011e207c7531017 fix use int32_t instead of int type for risczero compile with (-march=rv32i, -mabi=ilp32) (Simon) Pull request description: When compile bitcoin by the toolchain(`riscv32-unknown-elf-g++`) from risc0 , the compiler argument is `-march=rv32i, -mabi=ilp32`, which will get the error which due to not serialize the value of type int . ``` blockbody-guest: cargo:warning=In file included from depend/bitcoin/src/hash.h:14, blockbody-guest: cargo:warning= from depend/bitcoin/src/script/interpreter.h:9, blockbody-guest: cargo:warning= from depend/bitcoin/src/script/interpreter.cpp:6: blockbody-guest: cargo:warning=depend/bitcoin/src/serialize.h: In instantiation of 'void Serialize(Stream&, const T&) [with Stream = HashWriter; T = int]': blockbody-guest: cargo:warning=depend/bitcoin/src/hash.h:144:20: required from 'HashWriter& HashWriter::operator<<(const T&) [with T = int]' blockbody-guest: cargo:warning=depend/bitcoin/src/script/interpreter.cpp:1613:12: required from 'uint256 SignatureHash(const CScript&, const T&, unsigned int, int, const CAmount&, SigVersion, const PrecomputedTransactionData*) [with T = CTransaction; CAmount = long long int]' blockbody-guest: cargo:warning=depend/bitcoin/src/script/interpreter.cpp:1664:36: required from 'bool GenericTransactionSignatureChecker<T>::CheckECDSASignature(const std::vector<unsigned char>&, const std::vector<unsigned char>&, const CScript&, SigVersion) const [with T = CTransaction]' blockbody-guest: cargo:warning=depend/bitcoin/src/script/interpreter.cpp:1785:16: required from here blockbody-guest: cargo:warning=depend/bitcoin/src/serialize.h:776:7: error: request for member 'Serialize' in 'a', which is of non-class type 'const int' blockbody-guest: cargo:warning= 776 | a.Serialize(os); ``` -------------- ### Reason "The toolchain from RISC Zero defines int and int32_t as different types, although they have the same width. This means that `src/compat/assumptions.h` compiles fine; however, the templated serialization code cannot accept values of type int. Fix the compilation on RISC Zero by serializing int32_t instead of int values. This patch will explicitly use the `int32_t` type instead of `int` to avoid errors when compiling with the risc0 toolchain. Additionally, this patch will not change any behavior on platforms where compilation was previously successful. Fixes #30747 ACKs for top commit: maflcko: review-only ACK bc52cda1f3c007bdf1ed00aa3011e207c7531017 achow101: ACK bc52cda1f3c007bdf1ed00aa3011e207c7531017 TheCharlatan: ACK bc52cda1f3c007bdf1ed00aa3011e207c7531017 Tree-SHA512: ef880e7dfa1335bf2704ab17c0f506f17390b8259755674dfcd57131736492b2f4cfc36babda6902202b7c55a7513991e21f6634b0cd9b2b03baf4f1c0f8d78b
2024-09-20Merge bitcoin/bitcoin#30679: fix: handle invalid `-rpcbind` port earlierAva Chow
e6994efe08b282dd9e46602bcbad69567fe91dcd fix: increase rpcbind check robustness (tdb3) d38e3aed89eea665399eef07f5c3b64b14d4f586 fix: handle invalid rpcbind port earlier (tdb3) 83b67f2e6d59ea5de6573314ea4fe54ae52b7c12 refactor: move host/port checking (tdb3) 73c243965ab256ece089d14173c2d285955e83d5 test: add tests for invalid rpcbind ports (tdb3) Pull request description: Previously, when an invalid port was specified in `-rpcbind`, the `SplitHostPort()` return value in `HTTPBindAddresses()` was ignored and attempt would be made to bind to the default rpcbind port (with the host/port treated as a host). This rearranges port checking code in `AppInitMain()` to handle the invalid port before reaching `HTTPBindAddresses()`. Also adds a check in `HTTPBindAddresses()` as a defensive measure for future changes. Adds then updates associated functional tests as well. ACKs for top commit: achow101: ACK e6994efe08b282dd9e46602bcbad69567fe91dcd ryanofsky: Code review ACK e6994efe08b282dd9e46602bcbad69567fe91dcd zaidmstrr: Code review ACK [e6994ef](https://github.com/bitcoin/bitcoin/commit/e6994efe08b282dd9e46602bcbad69567fe91dcd) Tree-SHA512: bcc3e5ceef21963821cd16ce6ecb83d5c5657755882c05872a7cfe661a1492b1d631f54de22f41fdd173512d62dd15dc37e394fe1a7abe4de484b82cd2438b92
2024-09-20Merge bitcoin/bitcoin#30828: interfaces: #30697 follow upsAva Chow
84663291275248fd52da644b0c2566bbf9cc780b chain: simplify `deleteRwSettings` code and improve it's doc (ismaelsadeeq) f8d91f49c7091102138fcb123850399e8fadfbc7 chain: dont check for null settings value in `overwriteRwSetting` (ismaelsadeeq) df601993f2d7454f081316d6a8ddefbcefa49b3d chain: ensure `updateRwSetting` doesn't update to a null settings (ismaelsadeeq) c8e2eeeffb494d99d95c1c45efeda5a98dce94cd chain: uniformly use `SettingsAction` enum in settings methods (ismaelsadeeq) 1e9e735670f029c975d3b7486a54e5bb67135df7 chain: move new settings safely in `overwriteRwSetting` (ismaelsadeeq) 1c409004c80bc5f2314e20cc922076e22931cf73 test: remove wallet context from `write_wallet_settings_concurrently` (ismaelsadeeq) Pull request description: This PR addresses the remaining review comments from #30697 1. Disallowed overwriting settings values with a `null` value. 2. Uniformly used the `SettingsAction` enum in all settings methods instead of a boolean parameter. 3. Updated `overwriteRwSetting` to receive the `common::SettingsValue` parameter by value, enabling it to be moved safely. 4. Removed wallet context from the `write_wallet_settings_concurrently` unit test, as it is not needed. ACKs for top commit: achow101: ACK 84663291275248fd52da644b0c2566bbf9cc780b ryanofsky: Code review ACK 84663291275248fd52da644b0c2566bbf9cc780b. Looks good, thanks for taking suggestions and applying them to the right commits. Only changes since last review were documentation improvements and simplifying delete method. furszy: Code review ACK 84663291275248fd52da644b0c2566bbf9cc780b Tree-SHA512: baf2f59ed5aac4a4bda0c84fb6554a466a40d1f7b52b61dc2ff293d83ae60e82b925b7003237b633fecb65eba3a4c108e69166046895d1295809fbe0de67b052
2024-09-20Merge bitcoin/bitcoin#30568: addrman: change internal id counting to int64_tAva Chow
51f7668d31e2624e41c7ce77fe33162802808f3f addrman: change nid_type from int to int64_t (Martin Zumsande) 051ba3290e30e210bfc50dea974063053313ad3e addrman, refactor: introduce user-defined type for internal nId (Martin Zumsande) Pull request description: With `nIdCount` being incremented for each addr received, an attacker could cause an overflow in the past, see https://bitcoincore.org/en/2024/07/31/disclose-addrman-int-overflow/ Even though that attack was made infeasible indirectly by addr rate-limiting (PR #22387), to be on the safe side and prevent any regressions change the `nId`s used internally to `int64_t`. This is being done by first introducing a user-defined type for `nId`s in the first commit, and then updating it to `int64_t` (thanks sipa for help with this!). Note that `nId` is only used internally, it is not part of the serialization, so `peers.dat` should not be affected by this. I assume that the only reason this was not done in the past is to not draw attention to this previously undisclosed issue. ACKs for top commit: naumenkogs: ACK 51f7668d31e2624e41c7ce77fe33162802808f3f stratospher: ACK 51f7668d31e2624e41c7ce77fe33162802808f3f. I think it's a good change to make the nId space large(64 bits) so that the nId values are distinct. achow101: ACK 51f7668d31e2624e41c7ce77fe33162802808f3f Tree-SHA512: 68d4b8b0269a01a9544bedfa7c1348ffde00a288537e4c8bf2b88372ac7d96c4566a44dd6b06285f2fcf31b4f9336761e3bca7253fbc20db5e0d04e887156224
2024-09-20Merge bitcoin/bitcoin#26990: cli: Improve error message on multiwallet ↵Ava Chow
cli-side commands 54227e681a4efa8961f1ad05d43366d88a9b686a rpc, cli: improve error message on multiwallet mode (pablomartin4btc) Pull request description: Running a CLI command when multiple wallets are loaded and `-rpcwallet` is not specified, should return a clearer error. Currently in `master`: ``` $ bitcoin-cli -regtest -generate 1 error code: -19 error message: Wallet file not specified (must request wallet RPC through /wallet/<filename> uri-path). Try adding "-rpcwallet=<filename>" option to bitcoin-cli command line. ``` With this change: ``` $ bitcoin-cli -regtest -generate 1 error code: -19 error message: Multiple wallets are loaded. Please select which wallet to use by requesting the RPC through the /wallet/<walletname> URI path. Or for the CLI, specify the "-rpcwallet=<walletname>" option before the command (run "bitcoin-cli -h" for help or "bitcoin-cli listwallets" to see which wallets are currently loaded). ``` ACKs for top commit: maflcko: review ACK 54227e681a4efa8961f1ad05d43366d88a9b686a achow101: ACK 54227e681a4efa8961f1ad05d43366d88a9b686a furszy: utACK 54227e681a4 mzumsande: Code Review ACK 54227e681a4efa8961f1ad05d43366d88a9b686a jonatack: ACK 54227e681a4efa8961f1ad05d43366d88a9b686a Tree-SHA512: 51ff24f64858aa6be6adf6f20105c9f076ebea743780bf2a4399f7fe8b5239cbb1ea06d32b2ef5e850da2369abb0ef7a52c50c2b8f31f4ca90d3a486abc9b77e
2024-09-20wallet: migration, write best locator before unloading walletfurszy
2024-09-20wallet: Write best block to disk before backupFabian Jahr
This ensures that the best block is included in the backup which leads to a more consistent behavior when loading the backup.
2024-09-20Merge bitcoin/bitcoin#30561: refactor: move `SignSignature` helpers to test ↵merge-script
utils 58499b00d0ad1c83e433caa8fcc0e5d3fd3f2070 refactor: move `SignSignature` helpers to test utils (Sebastian Falbesoner) Pull request description: These helpers haven't been used in production code since segwit was merged more than eight years ago (see commit 605e8473, PR #8149), so it seems appropriate to move them to the test utils module. As suggested by instagibbs, see https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1697515508. ACKs for top commit: instagibbs: ACK 58499b00d0ad1c83e433caa8fcc0e5d3fd3f2070 pablomartin4btc: ACK 58499b00d0ad1c83e433caa8fcc0e5d3fd3f2070 Tree-SHA512: a52d3b92b477246f2ceb57c3690d0229a492b65a15dae331faeae9d96e5907f7fe1176edc1530243e0f088586984fd7ba435a0a2d2f2531c04d076fdf3f4095f
2024-09-20add check that chainwork doesn't exceed minimum workmarcofleon
2024-09-20add clarification in commentmarcofleon
2024-09-20Merge bitcoin/bitcoin#30856: build: drop obj/ subdirectory for generated build.hmerge-script
7025942687fd5e91d0a10ce5b2ac673b67a63491 build: drop superfluous `HAVE_BUILD_INFO` define (Sebastian Falbesoner) 0dd662510c52b202f7136fbcb32ed3864b52b50b build: drop obj/ subdir for generated build.h, rename to bitcoin-build-info.h (Sebastian Falbesoner) Pull request description: As indicated by the TODO, the obj subdirectory is not needed anymore now for the generated build.h header, since autotools are gone and we don't have in-source builds anymore (see #30454, #30664). In the second commit the superflous `HAVE_BUILD_INFO` macro is dropped, as suggested in https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2292424496. ACKs for top commit: theuni: utACK 7025942687fd5e91d0a10ce5b2ac673b67a63491 Tree-SHA512: 0a3b2cbbcf638344ceb74e5ba5a0fe2b1718427b23a18c8890258db36ce7177006a146178ed88d9c5ae956a5426f3844e86c1f4cca7c40946359742bffda983b
2024-09-19build: drop superfluous `HAVE_BUILD_INFO` defineSebastian Falbesoner
bitcoin-build-info.h should always be generated before clientversion.cpp is compiled due to the following explicit dependency in src/CMakeLists.txt: add_dependencies(bitcoin_clientversion generate_build_info) Hence there is no need to gate the inclusion of that header with an extra define.
2024-09-19build: drop obj/ subdir for generated build.h, rename to bitcoin-build-info.hSebastian Falbesoner
Now that this file is not in a subfolder anymore, prefix it with "bitcoin-" to avoid potential collisions. Also add "info" for a more descriptive name.
2024-09-19streams: reorder/document functionsPieter Wuille
2024-09-19streams: remove unused codePieter Wuille
2024-09-19Merge bitcoin/bitcoin#30889: log: Use ConstevalFormatStringmerge-script
facbcd4cef8890ae18976fb53b67ea56b3c04454 log: Use ConstevalFormatString (MarcoFalke) fae9b60c4ffef38d9725f42f992b1f38765312a3 test: Use LogPrintStr to test m_log_sourcelocations (MarcoFalke) fa39b1ca63874db8ef8bc16b87e2699e8e1b67be doc: move-only logging warning (MarcoFalke) Pull request description: This changes all logging (including the wallet logging) to produce a `ConstevalFormatString` at compile time, so that the format string can be validated at compile-time. I tested with `clang` and found that the compiler will use less than 1% more of time and memory. When an error is found, the compile-time error depends on the compiler, but it may look similar to: ``` src/util/string.h: In function ‘int main(int, char**)’: src/bitcoind.cpp:265:5: in ‘constexpr’ expansion of ‘util::ConstevalFormatString<1>(((const char*)"Hi %s %s"))’ src/util/string.h:38:98: in ‘constexpr’ expansion of ‘util::ConstevalFormatString<1>::Detail_CheckNumFormatSpecifiers(std::basic_string_view<char>(((const char*)((util::ConstevalFormatString<1>*)this)->util::ConstevalFormatString<1>::fmt)))’ src/util/string.h:78:34: error: expression ‘<throw-expression>’ is not a constant expression 78 | if (num_params != count) throw "Format specifier count must match the argument count!"; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` This refactor does not change behavior of the compiled executables. ACKs for top commit: hodlinator: re-ACK facbcd4cef8890ae18976fb53b67ea56b3c04454 l0rinc: ACK facbcd4cef8890ae18976fb53b67ea56b3c04454 ryanofsky: Code review ACK facbcd4cef8890ae18976fb53b67ea56b3c04454 pablomartin4btc: re-ACK facbcd4cef8890ae18976fb53b67ea56b3c04454 stickies-v: Approach ACK and code LGTM facbcd4cef8890ae18976fb53b67ea56b3c04454 modulo a `tinyformat::format_error` concern. Tree-SHA512: 852f74d360897020f0d0f6e5064edc5e7f7dacc2bec1d5feff22c634a2fcd2eb535aa75be0b7191d9053728be6108484c737154b02d68ad3186a2e5544ba0db8
2024-09-19test: generalize HasReason and use it in FailFmtWithErrorLÅ‘rinc
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
2024-09-18Merge bitcoin/bitcoin#30875: doc: fixed inconsistencies in documentation ↵merge-script
between autotools to cmake change a9964c04447745435747d9cc557165c43902783b doc: Updating docs from autotools to cmake (kevkevinpal) Pull request description: A bit of a followup from https://github.com/bitcoin/bitcoin/pull/30840 - In this change the documentation where we refer to the `./configure` script which is now gone and have converted the configure params to use the `cmake` equivalent. ACKs for top commit: maflcko: ACK a9964c04447745435747d9cc557165c43902783b jonatack: utACK a9964c04447745435747d9cc557165c43902783b jarolrod: ACK a9964c04447745435747d9cc557165c43902783b tdb3: ACK a9964c04447745435747d9cc557165c43902783b pablomartin4btc: re-ACK a9964c04447745435747d9cc557165c43902783b Tree-SHA512: f7ed20b8ad61f028c0d242b9cc70650d8da63057d4a8f7da88f0117a8d3241c5fe8fcf19d56ec82088160b9fee9b175fe9f64e5a845260d3696dc7e94bfdd0bd
2024-09-18doc: Updating docs from autotools to cmakekevkevinpal
replaced --enable-debug with -DCMAKE_BUILD_TYPE=Debug in developer-notes replaced --enable-multiprocess with -DWITH_MULTIPROCESS=ON replaced --disable-zmq with -DWITH_ZMQ=OFF
2024-09-17fix: increase rpcbind check robustnesstdb3
Adds invalid rpcbind port checking to `HTTPBindAddresses()`. While movement of `CheckHostPortOptions()` in the previous commit handles rcpbind port errors, updating `HTTPBindAddresses()` port checking adds a defensive measure for potential future changes.
2024-09-17fix: handle invalid rpcbind port earliertdb3
Previously, when an invalid port was specified in `-rpcbind`, the `SplitHostPort()` return value in `HTTPBindAddresses()` was ignored and attempt would be made to bind to the default rpcbind port (with the host/port treated as a host). This rearranges port checking code in `AppInitMain()` to handle the invalid port before reaching `HTTPBindAddresses()`. Also adjusts associated functional tests.
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-20 10:39:50 +0000