0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-09 10:43:19 -05:00
Commit graph

24447 commits

Author SHA1 Message Date
TheCharlatan
e4af2408f2
refactor: Pimpl leveldb::Iterator for CDBIterator
Hide the leveldb::Iterator member variable with a pimpl in order not to
expose it directly in the header.

Also, move CDBWrapper::NewIterator to the dbwrapper implementation to
use the pimpl for CDBIterator initialziation.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.
2023-08-05 10:42:53 +02:00
TheCharlatan
ef941ff128
refactor: Split dbwrapper CDBIterator::GetValue implementation
Keep the generic serialization in the header, while moving
leveldb-specifics to the implementation file.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.
2023-08-05 10:42:51 +02:00
TheCharlatan
b7a1ab5cb4
refactor: Split dbwrapper CDBIterator::GetKey implementation
Keep the generic serialization in the header, while moving
leveldb-specifics to the implementation file.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.
2023-08-05 10:42:48 +02:00
TheCharlatan
d7437908cd
refactor: Split dbwrapper CDBIterator::Seek implementation
Keep the generic serialization in the header, while moving
leveldb-specifics to the implementation file.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.
2023-08-05 10:42:45 +02:00
TheCharlatan
ea8135de7e
refactor: Pimpl leveldb::batch for CDBBatch
Hide the leveldb::WriteBatch member variable with a pimpl in order not
to expose it directly in the header.

Also move CDBBatch::Clear to the dbwrapper implementation to use the new
impl_batch.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.
2023-08-05 10:42:38 +02:00
TheCharlatan
b9870c920d
refactor: Split dbwrapper CDBatch::Erase implementation
Keep the generic serialization in the header, while moving
leveldb-specifics to the implementation file.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.
2023-08-05 10:27:53 +02:00
TheCharlatan
532ee812a4
refactor: Split dbwrapper CDBBatch::Write implementation
Keep the generic serialization in the header, while moving
leveldb-specifics to the implementation file.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.
2023-08-05 10:27:47 +02:00
TheCharlatan
afc534df9a
refactor: Wrap DestroyDB in dbwrapper helper
Wrap leveldb::DestroyDB in a helper function without exposing
leveldb-specifics.

Also, add missing optional include.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.
2023-08-01 22:14:15 +02:00
fanquake
42a9110899
Merge bitcoin/bitcoin#28162: refactor: Revert additional univalue check in ParseSighashString
06199a995f refactor: Revert addition of univalue sighash string check (TheCharlatan)
0b47c16215 doc: Correct release-notes for sighashtype exceptions (TheCharlatan)

Pull request description:

  This is a follow up for #28113.

  The string type check is already done by the rpc parser / RPCHelpMan. Re-doing it is adding dead code. Instead, throwing an exception when the assumption does not hold is the already correct behavior. Pointed out in this [comment](https://github.com/bitcoin/bitcoin/pull/28113/files#r1274568557).

  Also correct the release note for the correct sighashtype exception change. There is no change in the handling of non-string sighashtype arugments. Pointed out in this [comment](https://github.com/bitcoin/bitcoin/pull/28113/files#r1274567555).

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 06199a995f
  jonatack:
    Tested ACK 06199a995f
  stickies-v:
    ACK 06199a995f

Tree-SHA512: 3faa6b3d2247624c0973df8d79c09fbf1f90ffb99f1be484e359b528f485c31affea45976759bd206e4c81cbb54ebba5ad0ef4127d1deacbfe2a58153fcc94ee
2023-07-28 12:29:55 +01:00
fanquake
8aa77a77e6
Merge bitcoin/bitcoin#28168: refactor: Remove unused raw-pointer read helper from univalue
fa940f41ea Remove unused raw-pointer read helper from univalue (MarcoFalke)

Pull request description:

  The helpers are unused outside of tests and redundant with the existing `bool read(std::string_view raw);`.

  Fix both issues by removing them.

  Also, simplify the tests code by removing a `std::string` constructor where possible.

ACKs for top commit:
  stickies-v:
    utACK fa940f41ea
  TheCharlatan:
    tACK fa940f41ea

Tree-SHA512: 60c154c1046f01551335af79bf820a6104844f63e89977271b4336b3cd06ac3bab1379e18b7bc61d12bef7446029e91c16541ddecf9e88bc8bc897fc1f6ee2c8
2023-07-28 12:04:49 +01:00
Andrew Chow
cbf385058b
Merge bitcoin/bitcoin#27888: Fuzz: a more efficient descriptor parsing target
131314b62e fuzz: increase coverage of the descriptor targets (Antoine Poinsot)
90a24741e7 fuzz: add a new, more efficient, descriptor parsing target (Antoine Poinsot)
d60229ede5 fuzz: make the parsed descriptor testing into a function (Antoine Poinsot)

Pull request description:

  The current descriptor parsing fuzz target requires valid public or private keys to be provided. This is unnecessary as we are only interested in fuzzing the descriptor parsing logic here (other targets are focused on fuzzing keys serializations). And it's pretty inefficient, especially for formats that need a checksum (`xpub`, `xprv`, WIF).

  This introduces a new target that mocks the keys as an index in a list of precomputed keys. Keys are represented as 2 hex characters in the descriptor. The key type (private, public, extended, ..) is deterministically based on this one-byte value. Keys are deterministically generated at target initialization. This is much more efficient and also largely reduces the size of the seeds.
  TL;DR: for instance instead of requiring the fuzzer to generate a `pk(xpub6DdBu7pBoyf7RjnUVhg8y6LFCfca2QAGJ39FcsgXM52Pg7eejUHLBJn4gNMey5dacyt4AjvKzdTQiuLfRdK8rSzyqZPJmNAcYZ9kVVEz4kj)` to parse a valid descriptor, it just needs to generate a `pk(03)`.

  Note we only mock the keys themselves, not the entire descriptor key expression. As we want to fuzz the real code that parses the rest of the key expression (origin, derivation paths, ..).

  This is a target i used for reviewing #17190 and #27255, and figured it was worth PR'ing on its own since the added complexity for mocking the keys is minimal and it could help prevent introducing bugs to the descriptor parsing logic much more efficiently.

ACKs for top commit:
  MarcoFalke:
    re-ACK 131314b62e  🐓
  achow101:
    ACK 131314b62e

Tree-SHA512: 485a8d6a0f31a3a132df94dc57f97bdd81583d63507510debaac6a41dbbb42fa83c704ff3f2bd0b78c8673c583157c9a3efd79410e5e79511859e1470e629118
2023-07-27 13:48:12 -04:00
Andrew Chow
272c4f3f10
Merge bitcoin/bitcoin#28148: refactor: consistently use ApplyArgsManOptions for PeerManager::Options
8a3159728a refactor: deduplicate ignores_incoming_txs (stickies-v)
5f41afcc46 refactor: set ignore_incoming_txs in ApplyArgsManOptions (stickies-v)

Pull request description:

  Consistently use `ApplyArgsManOptions` for `PeerManager::Options`, and initialize `PeerManager::Options` early to avoid reading `"-blocksonly"` twice. Suggested in https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1268400386 and also requested in https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1273346189.

  No behaviour change, but the [`TestingSetup`](e35fb7bc48/src/test/util/setup_common.cpp (L255-L256)) is now also able to access `"-blocksonly"`.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 8a3159728a
  achow101:
    ACK 8a3159728a
  TheCharlatan:
    ACK 8a3159728a
  dergoegge:
    utACK 8a3159728a

Tree-SHA512: 6cb489d79ac2a87e8faedb76c96973ab3fc597426f274a90a3ffd0bc5fe3f2b25db9c7ec2e55a0c806c2bcbc0fdded6e228adb43d2cd81f14fd6552863847698
2023-07-27 11:33:47 -04:00
MarcoFalke
fa940f41ea
Remove unused raw-pointer read helper from univalue 2023-07-27 14:24:52 +02:00
TheCharlatan
06199a995f
refactor: Revert addition of univalue sighash string check
This check is already done by the rpc parser. Re-doing it is adding dead
code. Instead, throwing an exception when the assumption does not hold
is the already correct behavior.

To make the fuzz test more accurate and not swallow all runtime errors,
add a check that the passed in UniValue sighash argument is either a
string or null.

Co-authored-by: stickies-v <stickies-v@protonmail.com>
2023-07-27 09:36:05 +02:00
fanquake
f57e724a80
Merge bitcoin/bitcoin#28127: refactor: Remove C-style const-violating cast, Use reinterpret_cast
fa9108f85a refactor: Use reinterpret_cast where appropriate (MarcoFalke)
3333f950d4 refactor: Avoid casting away constness (MarcoFalke)
fa6394dd10 refactor: Remove unused C-style casts (MarcoFalke)

Pull request description:

  Using a C-style cast to convert pointer types to a byte-like pointer type has many issues:

  * It may accidentally and silently throw away `const`.
  * It forces reviewers to check that it doesn't accidentally throw away `const`.

  For example, on current master a `const char*` is cast to `unsigned char*` (without `const`), see d23fda0584/src/span.h (L273) . This can lead to UB, and the only reason why it didn't lead to UB is because the return type added back the `const`. (Obviously this would break if the return type was deduced via `auto`)

  Fix all issues by adding back the `const` and using `reinterpret_cast` where appropriate.

ACKs for top commit:
  darosior:
    re-utACK fa9108f85a
  hebasto:
    re-ACK fa9108f85a.
  john-moffett:
    ACK fa9108f85a

Tree-SHA512: 87f6e4b574f9bd96d4e0f2a0631fd0a9dc6096e5d4f1b95042fe9f197afc2fe9a24e333aeb34fed11feefcdb184a238fe1ea5aff10d580bb18d76bfe48b76a10
2023-07-26 16:03:39 +01:00
fanquake
c2ff87e1fa
Merge bitcoin/bitcoin#28150: test: Avoid intermittent issues due to async events in validationinterface_tests
faca9a3d5a test: Avoid intermittent issues due to async events in validationinterface_tests (MarcoFalke)

Pull request description:

  Currently the tests have many issues:

  * They setup the genesis block, even though it is not needed
  * They queue an async `UpdatedBlockTip` even, which causes intermittent issues: https://github.com/bitcoin/bitcoin/issues/28146#issuecomment-1650064645

  Fix all issues by trimming down the setup to just `ChainTestingSetup`.

ACKs for top commit:
  Crypt-iQ:
    tACK faca9a3d5a

Tree-SHA512: 4449040330f89bbaf5ce5b2052417c160b451c373987fdf1069596c07834ed81f0aea1506d53c7d2cd21062b27332d30679285dae194b272fd0cb9ce5ded32cf
2023-07-26 09:58:33 +01:00
Andrew Chow
32c15237b6
Merge bitcoin/bitcoin#27930: util: Don't derive secure_allocator from std::allocator
07c59eda00 Don't derive secure_allocator from std::allocator (Casey Carter)

Pull request description:

  Giving the C++ Standard Committee control of the public interface of your type means they will break it. C++23 adds a new `allocate_at_least` member to `std::allocator`. Very bad things happen when, say, `std::vector` uses `allocate_at_least` from `secure_allocator`'s base to allocate memory which it then tries to free with `secure_allocator::deallocate`.

  (Discovered by microsoft/STL#3712, which will be reverted by microsoft/STL#3819 before it ships.)

ACKs for top commit:
  jonatack:
    re-ACK 07c59eda00 no change since my previous ACK apart from squashing the commits
  achow101:
    ACK 07c59eda00
  john-moffett:
    ACK 07c59eda00 Reviewed and tested. Performance appears unaffected in my environment.

Tree-SHA512: 23606c40414d325f5605a9244d4dd50907fdf5f2fbf70f336accb3a2cb98baa8acd2972f46eab1b7fdec1d28a843a96b06083cd2d09791cda7c90ee218e5bbd5
2023-07-25 18:54:29 -04:00
Andrew Chow
1ed8a0f8d2
Merge bitcoin/bitcoin#28113: kernel: Remove UniValue from kernel library
6960c81cbf kernel: Remove Univalue from kernel library (TheCharlatan)
10eb3a9faa kernel: Split ParseSighashString (TheCharlatan)

Pull request description:

  Besides the build system changes, this is a mostly move-only change for moving the few UniValue-related functions out of kernel files.

  UniValue is not required by any of the kernel components and a JSON library should not need to be part of a consensus library.

ACKs for top commit:
  achow101:
    ACK 6960c81cbf
  theuni:
    Re-ACK 6960c81cbf
  stickies-v:
    re-ACK 6960c81cbf

Tree-SHA512: d92e4cb4e12134c94b517751bd746d39f9b8da528ec3a1c94aaedcce93274a3bae9277832e8a7c0243c13df0397ca70ae7bbb24ede200018c569f8d81103c1da
2023-07-25 18:13:16 -04:00
TheCharlatan
6960c81cbf
kernel: Remove Univalue from kernel library
It is not required by any of the kernel components.
A JSON library should not need to be part of a consensus library.
2023-07-25 17:40:07 +02:00
TheCharlatan
10eb3a9faa
kernel: Split ParseSighashString
This split is done in preparation for the next commit where the
dependency on UniValue in the kernel library is removed.
2023-07-25 17:40:02 +02:00
MarcoFalke
faca9a3d5a
test: Avoid intermittent issues due to async events in validationinterface_tests 2023-07-25 17:32:16 +02:00
stickies-v
8a3159728a
refactor: deduplicate ignores_incoming_txs
Initialize PeerManager::Options early to avoid reading -blocksonly twice.
2023-07-25 14:34:15 +01:00
stickies-v
5f41afcc46
refactor: set ignore_incoming_txs in ApplyArgsManOptions
Refactor to consistently use ApplyArgsManOptions to set all PeerManager::Options,
including ignore_incoming_txs.
2023-07-25 14:34:06 +01:00
Casey Carter
07c59eda00 Don't derive secure_allocator from std::allocator
Affects both secure_allocator and zero_after_free_allocator.

Giving the C++ Standard Committee control of the public interface of your type means they will break it. C++23 adds a new `allocate_at_least` member to `std::allocator`. Very bad things happen when, say, `std::vector` uses `allocate_at_least` from `secure_allocator`'s base to allocate memory which it then tries to free with `secure_allocator::deallocate`.

Drive-by: Aggressively remove facilities unnecessary since C++11 from both allocators to keep things simple.
2023-07-24 22:33:40 -07:00
dergoegge
23c7b51ddd [net processing] Move -capturemessages to PeerManager::Options 2023-07-24 18:35:30 +02:00
dergoegge
bd59bda26b [net processing] Move -blockreconstructionextratxn to PeerManager::Options 2023-07-24 18:35:30 +02:00
dergoegge
567c4e0b6a [net processing] Move -maxorphantx to PeerManager::Options 2023-07-24 18:35:30 +02:00
dergoegge
fa9e6d80d1 [net processing] Move -txreconciliation to PeerManager::Options 2023-07-24 18:35:28 +02:00
dergoegge
4cfb7b925f [net processing] Use ignore_incoming_txs from m_opts 2023-07-24 18:31:16 +02:00
dergoegge
8b87725921 [net processing] Introduce PeerManager options 2023-07-24 18:30:59 +02:00
MarcoFalke
fa9108f85a
refactor: Use reinterpret_cast where appropriate
Also, wrap reinterpret_cast into a CharCast to ensure it is only called
on byte pointers.
2023-07-24 15:32:35 +02:00
MarcoFalke
3333f950d4
refactor: Avoid casting away constness
Seems confusing and brittle to remove const and then add it back in the
return type.
2023-07-24 15:32:27 +02:00
MarcoFalke
fa6394dd10
refactor: Remove unused C-style casts 2023-07-24 15:32:00 +02:00
Antoine Poinsot
131314b62e
fuzz: increase coverage of the descriptor targets
Once a descriptor is successfully parsed, execute more of its methods.
There is probably still room for improvements by checking for some
invariants, but this is a low hanging fruit that significantly increases
the code coverage of these targets.
2023-07-21 19:14:36 +02:00
Antoine Poinsot
90a24741e7
fuzz: add a new, more efficient, descriptor parsing target
This new target focuses on fuzzing the actual descriptor parsing logic
by not requiring the fuzzer to produce valid keys (nor a valid checksum
for that matter).
This should make it much more efficient to find bugs we could introduce
moving forward.

Using a character as a marker (here '%') to be able to search and
replace in the string without having to mock the actual descriptor
parsing logic was an insight from Pieter Wuille.
2023-07-21 19:14:30 +02:00
Antoine Poinsot
d60229ede5
fuzz: make the parsed descriptor testing into a function
We'll be reusing it in the new target.
2023-07-21 10:40:13 +02:00
Andrew Chow
7edce77ff3
Merge bitcoin/bitcoin#28067: descriptors: do not return top-level only funcs as sub descriptors
dd9633b516 test: wallet, add coverage for watch-only raw sh script migration (furszy)
cc781a2180 descriptor: InferScript, do not return top-level only func as sub descriptor (furszy)
286e0c7d5e wallet: loading, log descriptor parsing error details (furszy)

Pull request description:

  Linked to #28057.

  Currently, the `InferScript` function returns an invalid descriptor when it tries to infer a p2sh-p2pkh script whose pubkey is not known by the wallet.

  This behavior occurs because the inference process bypasses the `pkh` subscript when the pubkey is not contained by the wallet (no pubkey provider), interpreting it as a `sh(addr(ADDR))` descriptor. Then, the failure arises because the `addr()` function is restricted to being used only at the top level.

  For reviewers, would recommend to start by examining the functional test to understand the context and the circumstances on which this can result in a fatal error (e.g. during the migration process).

ACKs for top commit:
  achow101:
    ACK dd9633b516
  darosior:
    utACK dd9633b516

Tree-SHA512: 61e763206c604c372019d2c36e31684f3dddf81f8b154eb9aba5cd66d8d61bda457ed4e591613eb6ce6c76cf7c3f11764abc6cd727a7c2b6414f1065783be032
2023-07-20 11:16:45 -04:00
furszy
cc781a2180
descriptor: InferScript, do not return top-level only func as sub descriptor
e.g. sh(addr(ADDR)) or sh(raw(HEX)) are invalid descriptors.

Making sh and wsh top level functions to return addr/raw descriptors when
the subscript inference fails.
2023-07-20 11:04:52 -03:00
fanquake
ac7c1772f9
Merge bitcoin/bitcoin#26654: util: Show descriptive error messages when FileCommit fails
5408a55fc8 Consolidate Win32-specific error formatting (John Moffett)
c95a4432d7 Show descriptive error messages when FileCommit fails (John Moffett)

Pull request description:

  Only raw [`errno`](https://en.cppreference.com/w/cpp/error/errno) int values are logged if `FileCommit` fails. These values are implementation-specific, so it makes it harder to debug based on user reports. For instance, https://github.com/bitcoin/bitcoin/issues/26455#issue-1436654238 and [another](https://bitcointalk.org/index.php?topic=5182526.0#:~:text=FileCommit%3A%20FlushFileBuffers%20failed%3A%205).

  Instead, use `SysErrorString` (or the refactored Windows equivalent `Win32ErrorString`) to display both the raw int value and the descriptive message. All other instances in the code I could find where `errno` or (Windows-only) `GetLastError()`/`WSAGetLastError()` are logged use the full descriptive string. For example:

  1b680948d4/src/util/sock.cpp (L390)

  1b680948d4/src/util/sock.cpp (L272)

  7e1007a3c6/src/netbase.cpp (L515-L516)

  8ccab65f28/src/init.cpp (L164)

  I refactored the Windows formatting code to put it in `syserror.cpp`, as it's applicable to all Win32 API system errors, not just networking errors. To be clear, the Windows API functions `WSAGetLastError()` and `GetLastError()` are currently [equivalent](https://stackoverflow.com/questions/15586224/is-wsagetlasterror-just-an-alias-for-getlasterror).

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 5408a55fc8 💡

Tree-SHA512: 3921cbac98bd9edaf84d3dd7a43896c7921f144c8ca2cde9bc96d5fb05281f7c55e7cc99db8debf6203b5f916f053025e4fa741f51458fe2c53bb57b0a781027
2023-07-20 13:37:21 +01:00
fanquake
355bbcba01
Merge bitcoin/bitcoin#28066: fuzz: Generate process_message targets individually
fa6245da60 fuzz: Generate process_message targets individually (MarcoFalke)
fa1471e575 refactor: Remove duplicate allNetMessageTypesVec (MarcoFalke)

Pull request description:

  Now that `LIMIT_TO_MESSAGE_TYPE` is a runtime setting after commit 927b001502, it shouldn't hurt to also generate each message type individually. Something similar was done for the `rpc` target in commit cf4da5ec29.

ACKs for top commit:
  stickies-v:
    re-crACK fa6245da60
  brunoerg:
    reACK fa6245da60

Tree-SHA512: 8f3ec71bab89781f10820a0e027fcde8949f3333eb19a30315aaad6f90f5167028113cea255b2d60b700da817c7eaac20b7b4c92f931052d7f5c2f148d33aa5a
2023-07-20 10:17:08 +01:00
fanquake
04afe55e29
Merge bitcoin/bitcoin#26467: bumpfee: Allow the user to choose which output is change
e8c31f135c tests: Test for bumping single output transaction (Andrew Chow)
4f4d4407e3 test: Test bumpfee reduce_output (Andrew Chow)
7d83502d3d bumpfee: Allow original change position to be specified (Andrew Chow)

Pull request description:

  When bumping the transaction fee, we will try to find the change output of the transaction in order to have an output whose value we can modify so that we can meet the target feerate. However we do not always find the change output which can cause us to unnecessarily add an additional output to the transaction. We can avoid this issue by outsourcing the determination of change to the user if they so desire.

  This PR adds a `orig_change_pos` option to bumpfee which the user can use to specify the index of the change output.

  Fixes #11233
  Fixes #20795

ACKs for top commit:
  ismaelsadeeq:
    re ACK e8c31f135c
  pinheadmz:
    re-ACK e8c31f135c
  furszy:
    Code review ACK e8c31f13

Tree-SHA512: 3a230655934af17f7c1a5953fafb5ef0d687c21355cf284d5e98fece411f589cd69ea505f06d6bdcf82836b08d268c366ad2dd30ae3d71541c9cdf94d1f698ee
2023-07-20 09:55:04 +01:00
Ryan Ofsky
5608e1d3b4
Merge bitcoin/bitcoin#27928: test: Add more tests for the BIP21 implementation
f1d807e383 Add more tests for the BIP21 implementation (Kiminuo)

Pull request description:

  This PR is an attempt to make it clear how the current BIP21 implementation behaves in Bitcoin Core. Especially, I'm interested whether one can specify multiple `amount` (`message`, etc.) parameters.

  My primary end goal is to answer [this question of mine](https://bitcoin.stackexchange.com/questions/118654/how-to-interpret-bip21-uri-with-amount-specified-twice/) but I figured that maybe it's worth a PR. If not, I'll close the PR.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK f1d807e383
  kevkevinpal:
    ACK [f1d807e](f1d807e383)

Tree-SHA512: d287809d47c5cfc667f850927bfd969bd345a996d3d53a4c26ef0ffd29eb75ef53358692a15f9a0493ec9e1c101123b6584572e25f87bcb98ff67f6b6c166de4
2023-07-19 16:43:12 -04:00
Andrew Chow
4d828ef427
Merge bitcoin/bitcoin#28085: refactor: use Span for SipHash::Write
7d92b1430a refactor: use Span for SipHash::Write (Sebastian Falbesoner)

Pull request description:

  This simple refactoring PR changes the interface for the `SipHash` arbitrary-data `Write` method to take a `Span<unsigned char>` instead of having to pass data and length. (`Span<std::byte>` seems to be more modern, but vectors of `unsigned char` are still used prety much everywhere where SipHash is called, and I didn't find it very appealing having to clutter the code with `Make(Writable)ByteSpan` helpers).

ACKs for top commit:
  sipa:
    utACK 7d92b1430a
  MarcoFalke:
    lgtm ACK 7d92b1430a
  achow101:
    ACK 7d92b1430a

Tree-SHA512: f17a27013c942aead4b09f5a64e0c3ff8dbc7e83fe63eb9a2e3ace8be9921c9cbba3ec67e3e83fbe3332ca941c42370efd059e702c060f9b508307e9657c66f2
2023-07-19 16:27:08 -04:00
fanquake
3648a9b4f7
Merge bitcoin/bitcoin#28056: rpc: doc: Added longpollid and data params to template_request
f6a26196cf Added `longpollid` and `data` params to `template_request` #27998 (Rhythm Garg)

Pull request description:

  This PR will add the optional parameters `longpollid` and `data` to `template_request` as they were missing when calling `help getblocktemplate` in RPCHelpMan.

  I request the maintainers to review this and let me know about any mistakes in the descriptions of the parameters.

  This PR refers to the issue #27998

ACKs for top commit:
  ItIsOHM:
    > tACK [f6a2619](f6a26196cf)
  russeree:
    tACK f6a26196cf
  stickies-v:
    tACK f6a26196cf

Tree-SHA512: 6c592db59cb11b2d031ce5265c547fa296266278f6c25f96afe18a420e0d547f4d483e0f66de75d52c0c319ac1585f3558b9f70c12ef208c96ec96a51f786c6a
2023-07-19 12:26:09 +01:00
fanquake
84c5416b03
Merge bitcoin/bitcoin#28093: subtree: update libsecp256k1 to latest master
5080c9c25f build: adapt Windows builds for libsecp256k1 build changes (fanquake)
ff061fde18 Squashed 'src/secp256k1/' changes from 705ce7ed8c..c545fdc374 (fanquake)

Pull request description:

  Includes https://github.com/bitcoin-core/secp256k1/pull/1378. Which fixes #28079.
  Adapts Windows build for https://github.com/bitcoin-core/secp256k1/pull/1367.

ACKs for top commit:
  hebasto:
    ACK 5080c9c25f, I've made the `src/secp256k1` subtree update locally and got zero diff with this PR branch.
  jonasnick:
    ACK 5080c9c25f

Tree-SHA512: 37915d420ebacefc6bc82c2511bff3d6884e01d5c92795f19cd61862f96b30aa1fe768aeabec128c9d25c1d8bc62b46b46969626067266074b39566ad9e2f5ba
2023-07-19 11:36:30 +01:00
fanquake
24d5cf9314
Merge bitcoin/bitcoin#27425: test: move remaining rand code from util/setup_common to util/random
1cd45d4e08 test: move random.h include header from setup_common.h to cpp (Jon Atack)
1b246fdd14 test: move remaining random test util code from setup_common to random (jonatack)

Pull request description:

  and drop the `util/random` dependency on `util/setup_common`.  This improves code separation and allows `util/setup_common` to call `util/random` functions without creating a circular dependency, thereby addressing https://github.com/bitcoin/bitcoin/pull/26940#issuecomment-1497266140 by glozow (thanks!)

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 1cd45d4e08 🌂

Tree-SHA512: 6ce63d9103ba9b04eebbd8ad02fe9aa79e356296533404034a1ae88e9b7ca0bc9a5c51fd754b71cf4e7b55b18bcd4d5474b2d588edee3851e3b3ce0e4d309a93
2023-07-19 10:26:11 +01:00
fanquake
8ee662984f
Update secp256k1 subtree to latest upstream master 2023-07-18 15:25:05 +01:00
fanquake
ff061fde18 Squashed 'src/secp256k1/' changes from 705ce7ed8c..c545fdc374
c545fdc374 Merge bitcoin-core/secp256k1#1298: Remove randomness tests
b40e2d30b7 Merge bitcoin-core/secp256k1#1378: ellswift: fix probabilistic test failure when swapping sides
c424e2fb43 ellswift: fix probabilistic test failure when swapping sides
907a67212e Merge bitcoin-core/secp256k1#1313: ci: Test on development snapshots of GCC and Clang
0f7657d59c Merge bitcoin-core/secp256k1#1366: field: Use `restrict` consistently in fe_sqrt
cc55757552 Merge bitcoin-core/secp256k1#1340: clean up in-comment Sage code (refer to secp256k1_params.sage, update to Python3)
600c5adcd5 clean up in-comment Sage code (refer to secp256k1_params.sage, update to Python3)
981e5be38c ci: Fix typo in comment
e9e9648219 ci: Reduce number of macOS tasks from 28 to 8
609093b387 ci: Add x86_64 Linux tasks for gcc and clang snapshots
1deecaaf3b ci: Install development snapshots of gcc and clang
b79ba8aa4c field: Use `restrict` consistently in fe_sqrt
c9ebca95f9 Merge bitcoin-core/secp256k1#1363: doc: minor ellswift.md updates
afd7eb4a55 Merge bitcoin-core/secp256k1#1371: Add exhaustive tests for ellswift (with create+decode roundtrip)
2792119278 Add exhaustive test for ellswift (create+decode roundtrip)
c7d900ffd1 doc: minor ellswift.md updates
332af315fc Merge bitcoin-core/secp256k1#1344: group: save normalize_weak calls in `secp256k1_ge_is_valid_var`/`secp256k1_gej_eq_x_var`
9e6d1b0e9b Merge bitcoin-core/secp256k1#1367: build: Improvements to symbol visibility logic on Windows (attempt 3)
0aacf64352 Merge bitcoin-core/secp256k1#1370: Corrected some typos
b6b9834e8d small fixes
07c0e8b82e group: remove unneeded normalize_weak in `secp256k1_gej_eq_x_var`
3fc1de5c55 Merge bitcoin-core/secp256k1#1364: Avoid `-Wmaybe-uninitialized` when compiling with `gcc -O1`
fb758fe8d6 Merge bitcoin-core/secp256k1#1323: tweak_add: fix API doc for tweak=0
c6cd2b15a0 ci: Add task for static library on Windows + CMake
020bf69a44 build: Add extensive docs on visibility issues
0196e8ade1 build: Introduce `SECP256k1_DLL_EXPORT` macro
9f1b1904a3 refactor: Replace `SECP256K1_API_VAR` with `SECP256K1_API`
ae9db95cea build: Introduce `SECP256K1_STATIC` macro for Windows users
7966aee31d Merge bitcoin-core/secp256k1#1369: ci: Print commit in Windows container
a7bec34231 ci: Print commit in Windows container
249c81eaa3 Merge bitcoin-core/secp256k1#1368: ci: Drop manual checkout of merge commit
98579e297b ci: Drop manual checkout of merge commit
5b9f37f136 ci: Add `CFLAGS: -O1` to task matrix
a6ca76cdf2 Avoid `-Wmaybe-uninitialized` when compiling with `gcc -O1`
0fa84f869d Merge bitcoin-core/secp256k1#1358: tests: introduce helper for non-zero `random_fe_test()` results
5a95a268b9 tests: introduce helper for non-zero `random_fe_test` results
304421d57b tests: refactor: remove duplicate function `random_field_element_test`
3aef6ab8e1 Merge bitcoin-core/secp256k1#1345: field: Static-assert that int args affecting magnitude are constant
4494a369b6 Merge bitcoin-core/secp256k1#1357: tests: refactor: take use of `secp256k1_ge_x_on_curve_var`
799f4eec27 Merge bitcoin-core/secp256k1#1356: ci: Adjust Docker image to Debian 12 "bookworm"
c862a9fb49 ci: Adjust Docker image to Debian 12 "bookworm"
a1782098a9 ci: Force DWARF v4 for Clang when Valgrind tests are expected
7d8d5c86df tests: refactor: take use of `secp256k1_ge_x_on_curve_var`
8a7273465b Help the compiler prove that a loop is entered
fd491ea1bb Merge bitcoin-core/secp256k1#1355: Fix a typo in the error message
ac43613d25 Merge bitcoin-core/secp256k1#1354: Add ellswift to CHANGELOG
67887ae65c Fix a typo in the error message
926dd3e962 Merge bitcoin-core/secp256k1#1295: abi: Use dllexport for mingw builds
10836832e7 Merge bitcoin-core/secp256k1#1336: Use `__shiftright128` intrinsic in `secp256k1_u128_rshift` on MSVC
7c7467ab7f Refer to ellswift.md in API docs
c32ffd8d8c Add ellswift to CHANGELOG
3c1a0fd37f Merge bitcoin-core/secp256k1#1347: field: Document return value of fe_sqrt()
5779137457 field: Document return value of fe_sqrt()
be8ff3a02a field: Static-assert that int args affecting magnitude are constant
efa76c4bf7 group: remove unneeded normalize_weak in `secp256k1_ge_is_valid_var`
5b7bf2e9d4 Use `__shiftright128` intrinsic in `secp256k1_u128_rshift` on MSVC
05873bb6b1 tweak_add: fix API doc for tweak=0
6ec3731e8c Simplify test PRNG implementation
fb5bfa4eed Add static test vector for Xoshiro256++
723e8ca8f7 Remove randomness tests
bc7c8db179 abi: Use dllexport for mingw builds

git-subtree-dir: src/secp256k1
git-subtree-split: c545fdc374964424683d9dac31a828adedabe860
2023-07-18 15:25:05 +01:00
fanquake
673acab223
Merge bitcoin/bitcoin#28090: validation: use noexcept instead of deprecated throw()
047daad4f5 clang-tidy: turn on modernize-use-noexcept (fanquake)
85e9e1f802 validation: use noexcept instead of deprecated throw() (fanquake)

Pull request description:

  We fixed this once before in https://github.com/bitcoin/bitcoin/pull/10965.
  Turn on https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-noexcept.html#modernize-use-noexcept.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 047daad4f5
  sipa:
    utACK 047daad4f5
  Empact:
    utACK 047daad4f5
  stickies-v:
    ACK 047daad4f5

Tree-SHA512: 949b0fe598d66583747853094db13f196b402000e601f8634e5a708b55454d29c5aa18eaf1f2420d3ccf10e3e524b7414ff3a6fe4cb431420bf749c22b2b8bab
2023-07-18 10:24:13 +01:00
Andrew Chow
bc88f3ab90
Merge bitcoin/bitcoin#27997: Descriptors: rule out unspendable miniscript descriptors
c7db88af71 descriptor: assert we never parse a sane miniscript with no pubkey (Antoine Poinsot)
a49402a9ec qa: make sure we don't let unspendable Miniscript descriptors be imported (Antoine Poinsot)
639e3b6c97 descriptor: refuse to parse unspendable miniscript descriptors (Antoine Poinsot)
e3280eae1b miniscript: make GetStackSize() and GetOps() return optionals (Antoine Poinsot)

Pull request description:

  `IsSane()` in Miniscript does not ensure a Script is actually spendable. This is an issue as we would accept any sane Miniscript when parsing a descriptor. Fix this by explicitly checking a Miniscript descriptor is both sane and spendable when parsing it.

  This bug was exposed due to a check added in #22838 (https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1226859880) that triggered a fuzz crash (https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-1612510057).

ACKs for top commit:
  sipa:
    utACK c7db88af71
  achow101:
    ACK c7db88af71

Tree-SHA512: e79bc9f7842e98a4e8f358f05811fca51b15b4b80a171c0d2b17cf4bb1f578a18e4397bc2ece9817d392e0de0196ee6a054b7318441fd3566dd22e1f03eb64a5
2023-07-17 19:16:09 -04:00