Instead of having the caller have to figure out the correct sane default
to provide to FillPSBT, have FillPSBT do that by having it take the
sighash type as an optional. This further allows it to distinguish
between an explicit sighash type being provided and expecting the
default value to be used.
SignPSBTInput will need to report the specific things that caused an
error to callers, so change it to return a PSBTError. Additionally some
callers will now check the return value and report an error to the user.
Currently, this should not change any behavior as the things that
SignPBSTInput will error on are all first checked by its callers.
4b241867567203b204823a4558c2aa5767acf028 test: add test for decoding PSBT with MuSig2 PSBT key types (BIP 373) (Sebastian Falbesoner)
8ba245cb8318c9be375ac2fb2a3e7ded875b4408 test: add constants for MuSig2 PSBT key types (BIP 373) (Sebastian Falbesoner)
Pull request description:
This PR is a follow-up to #31247 (see https://github.com/bitcoin/bitcoin/pull/31247#pullrequestreview-2427834909) and adds a functional test for decoding PSBTs (using the `decodepsbt` RPC) with MuSig2 per-input and per-output types. The first commit adds the new MuSig2 key types to the test frameworks and extends the PSBT serialization to cope with lists of bytestrings.
ACKs for top commit:
achow101:
ACK 4b241867567203b204823a4558c2aa5767acf028
rkrux:
re-ACK 4b24186
Tree-SHA512: f12919f71b3fff74df1d7ddaa8db455b1b139f7abd51d7f3fa5d750fc7dd613454b438c4e0dedad679476d414fa1da43ef1121e486b0bdfd97d5ef8bdf37f060
62fc42d475df4f23bd93313f95ee7b7eb0d4683f interfaces: refactor: move `waitTipChanged` implementation to miner (ismaelsadeeq)
c39ca9d4f7bc9ca155692ac949be2e61c0598a97 interfaces: move getTip implementation to miner (Sjors Provoost)
720f201e652885b9e0aec8e62a1bf9590052b320 interfaces: refactor: move `waitNext` implementation to miner (ismaelsadeeq)
e6c2f4ce7a841153510971f0236c527d1a499649 interfaces: refactor: move `submitSolution` implementation to miner (ismaelsadeeq)
02d4bc776bbe002ee624ec2c09d7c3f981be1b17 interfaces: remove redundant coinbase fee check in `waitNext` (ismaelsadeeq)
Pull request description:
#### Motivation
In [Internal interface guidelines](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#internal-interface-guidelines)
It's stated that
> Interface method definitions should wrap existing functionality instead of implementing new functionality. Any substantial new node or wallet functionality should be implemented in [src/node/](https://github.com/bitcoin/bitcoin/blob/master/src/node) or [src/wallet/](https://github.com/bitcoin/bitcoin/blob/master/src/wallet) and just exposed in [src/interfaces/](https://github.com/bitcoin/bitcoin/blob/master/src/interfaces) instead of being implemented there, so it can be more modular and accessible to unit tests.
However the some methods in the newly added `BlockTemplateImpl` and `MinerImpl` classes partially enforces this guideline, as the implementations of the `submitSolution`, `waitNext`, and `waitTipChanged` methods reside within the class itself.
#### What the PR Does
This PR introduces a simple refactor by moving certain method implementations from `BlockTemplateImpl` into the miner module. It introduces three new functions:
1. Remove rundundant coinbase fee check in `waitNext`
2. **`AddMerkleRootAndCoinbase`**: Computes the block's Merkle root, inserts the coinbase transaction, and sets the Merkle root in the block. This function is called by `submitSolution` before the block is submitted for processing.
3. **`WaitAndCreateNewBlock`**: Returns a new block template either when transaction fees reach a certain threshold or when a new tip is detected. If a timeout is reached, it returns `nullptr`. The `waitNext` method in `BlockTemplateImpl` now simply wraps this function.
4. Move `GetTip` implementation to miner.
5. **`WaitTipChanged`**: Returns the tip when the chain it changes, or `nullopt` if a timeout or interrupt occurs. The `waitTipChanged` method in `MinerImpl` now calls `GetTip` after invoking `ChainTipChanged`, and returns the tip.
#### Behavior Change
- We now only `Assert` for a valid chainman and notifications pointer once.
ACKs for top commit:
achow101:
ACK 62fc42d475df4f23bd93313f95ee7b7eb0d4683f
Sjors:
ACK 62fc42d475df4f23bd93313f95ee7b7eb0d4683f
ryanofsky:
Code review ACK 62fc42d475df4f23bd93313f95ee7b7eb0d4683f. Lots of suggest suggest changes made since last review, altering function names and signatures and also adding new commit to drop negative fee handling. I like the idea of making the wait function return a BlockRef, that is clearer than what I suggested. Left some comments below but they are not important and this looks good as-is
Tree-SHA512: 502632f94ced81f576b2c43cf015f1527e2c259e6ca253f670f5a6889171e2246372b4e709575701afa3f01d488d6633557fef54f48fe83bbaf1836ac5326c4f
a04f17a1882407db09b0a07338e12877ac1d9e92 doc: warn that CheckBlock() underestimates sigops (Sjors Provoost)
Pull request description:
Counting sigops in the witness requires context that `CheckBlock()` does not have, so it only counts sigops for non-segwit transactions.
It's useful to document, but it should not be a problem.
The commit message contains some historical context.
ACKs for top commit:
ismaelsadeeq:
ACK a04f17a1882407db09b0a07338e12877ac1d9e92
ryanofsky:
Code review ACK a04f17a1882407db09b0a07338e12877ac1d9e92
Tree-SHA512: 26528367a7f3cfa8540ef0b90f7aa912c8f0bc057428f20a1fd1d4e232dac77747bc20044f0fcb0ffab8a2e1fb3dbe3dab46be749553a917744ddc7a829025cb
- This commit creates a function `WaitTipChanged` that waits for the connected
tip to change until timeout elapsed.
- This function is now used by `waitTipChanged`
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
e7ad86e1ca3b0b2f2795e91c2f9959486c67dd90 test: fix another intermittent failure in wallet_basic.py (Martin Zumsande)
07350e204dedfba20da461d9cdcd469dc95e01c3 test: Fix intermittent failure in wallet_basic.py (Martin Zumsande)
Pull request description:
Fixes two rare failures that happened in the CI:
#27249:
There could be a race with outstanding TxAddedToMempool notifications being applied to the soon-to-be created wallet:
1. importdescriptors during rescan sets status to `TxStateConfirmed`
2. old `transactionAddedToMempool` notification changes status back to `TxStateInMempool`
3. If the listunspent call happens here the test will fail
4. blockConnected notification will change the status back to `TxStateConfirmed` (so it's not a persistent failure)
I could reproduce this by adding a 100 microsecond sleep to `AddToWallet()`, the fix is to add a sync, so `transactionAddedToMempool` notifications won't affect the new wallet anymore.
#32456:
During init, the test framework will start using rpc after the mempool was loaded.
It will not wait for `start()` / `postInitProcess` or outstanding `transactionAddedToMempool` notifications (which would both set the status to `TxStateInMempool`), leading to
a possible race, in which `listunspent` can be called while the tx is still in `Inactive` status.
Can be reproduced by adding two sleeps: To init before calling `start()` for the chain clients, plus to `transactionAddedToMempool` in `wallet.cpp`.
Prevent this by processing outstanding notifications.
Fixes#27249Fixes#32456
ACKs for top commit:
maflcko:
review ACK e7ad86e1ca3b0b2f2795e91c2f9959486c67dd90 🎩
Tree-SHA512: 1f1a11e5c8e1c6d3c39a49401c2c5122befdbbec25c0451953f5bfe8dfb53221ada552a68006e266570addda12bb16c1b9b1e49ad2198c33d91c4b96b764d73e
fa427ffceeefd368a1ade273501ce4b01133ad4d fuzz: Properly setup wallet in wallet_fees target (MarcoFalke)
Pull request description:
`g_wallet_ptr` is destructed after the `testing_setup`. This is not supported and will lead to issues such as https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2863875857 or https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2855259932.
This could be fixed by fixing the initialization order.
However, the global wallet is also modified in the fuzz target, which is bad fuzzing practise.
So instead fix it by constructing a fresh wallet for each fuzz iteration.
ACKs for top commit:
brunoerg:
code review ACK fa427ffceeefd368a1ade273501ce4b01133ad4d
hebasto:
ACK fa427ffceeefd368a1ade273501ce4b01133ad4d, this change fixes the issue when building the "Debug" configuration with MSVC on Windows.
marcofleon:
Code review ACK fa427ffceeefd368a1ade273501ce4b01133ad4d
Tree-SHA512: 161b93fc39a609cb16d9ffea7366c5e339bd01712577f0782aedff46c00f79edd2a907807ac83f9fcec687b4bbbe0fd6e6f75e32169639a310e4e7b771078b3b
1e0de7a6ba926487c8a075856b74af2a3a0eb8ef fees: document non-monotonic estimation edge case (willcl-ark)
Pull request description:
Closes: https://github.com/bitcoin/bitcoin/issues/11800
In scenarios where data is available for higher targets but not for lower ones, this method *may* return lower fee rates for higher confirmation targets. This could occur if `estimateCombinedFee` returns no valid data (`-1`) for some estimates for a low target, but **does** return valid data for a higher target.
Users of this function should be aware of this potential, if unlikely, inconsistency in behaviour in data-sparse scenarios.
ACKs for top commit:
adamandrews1:
Code review ACK 1e0de7a
ismaelsadeeq:
Code review ACK 1e0de7a6ba926487c8a075856b74af2a3a0eb8ef
glozow:
ACK 1e0de7a6ba926487c8a075856b74af2a3a0eb8ef
Tree-SHA512: 161e5dafdd131570853a89491753ae39a7b725d1a86cab5a7294c2a5939da1a9a5f2c4aca0900e9ad810e828b6e0e636f256384e3d1fda6dd552da189bbbe747
0750249289c092fc8e2e29669fec73a58b873767 mining: document gbt_rule_value helper (Sjors Provoost)
5e87c3ec094d68a7a27dfb7ae665b225ff4dfdb6 scripted-diff: rename gbt_force and gbt_force_name (Sjors Provoost)
Pull request description:
The term "force" is ambiguous and not used in [BIP9](https://github.com/bitcoin/bips/blob/master/bip-0009.mediawiki#getblocktemplate-changes) where there ! rule prefix is introduced.
E.g. this code is hard to read:
```cpp
if (!gbt_force) {
s.insert(s.begin(), '!');
```
Additionally, #29039 renamed `gbt_vb_name` to `gbt_force_name` which, at least for me, further increased the confusion.
This is a pure (variable rename) refactor (plus documentation) and does not change behavior.
Reminder of how to verify a scripted diff:
```sh
test/lint/commit-script-check.sh origin/master..HEAD
```
ACKs for top commit:
achow101:
ACK 0750249289c092fc8e2e29669fec73a58b873767
janb84:
ACK [0750249](0750249289)
musaHaruna:
ACK [0750249](0750249289)
glozow:
ACK 0750249289c092fc8e2e29669fec73a58b873767, seems sensible
Tree-SHA512: 8c88a273a3b36040f6c641843bd20579d0065b051aad4b39fc14f0d2af2808690dff6772bd8b1a4d9699b72279a700d2661012651bc315433a123dcc8996adaa
2aa63d511affdcc9980b58fc4ff18b8ad10b0f8c test: Use uninvolved pruned node in feature_pruning undo test (enoch)
772ba7f9ce09e836a51636524a8a96a23946d658 test: Fix nTimes typo in feature_pruning test (enoch)
Pull request description:
This PR contains two commits:
1. Fixes a typo in feature_pruning.py where 'nTimes' was incorrectly
used instead of 'nTime'. This typo caused the test to always reset
mine_large_blocks.nTime to 0, rather than only on the first run.
2. Fixes the test failure exposed by the typo fix. The
test_pruneheight_undo_presence test was failing because it was using
node 2, which is involved in reorg testing and could be on a
different chain than other nodes. The solution switches to using
node 5, which is also a pruned node but isn't involved in reorg
testing.
Testing:
- Ran test/functional/feature_pruning.py multiple times to verify
consistent passing
- Verified that the test now passes with the correct nTime variable name
- Confirmed the test behavior matches the intended functionality of
verifying pruned block availability
- Ran the full test suite to ensure the changes did not introduce any
regressions or affect other tests
Thanks to fjahr for his assistance in diagnosing the issue and
suggesting the solution.
This fixes the test failure reported in #32249
ACKs for top commit:
fjahr:
tACK 2aa63d511affdcc9980b58fc4ff18b8ad10b0f8c
maflcko:
lgtm ACK 2aa63d511affdcc9980b58fc4ff18b8ad10b0f8c
naiyoma:
tACK 2aa63d511affdcc9980b58fc4ff18b8ad10b0f8c
stratospher:
tested ACK 2aa63d5. verified that `nTime` is being incremented now.
Tree-SHA512: a543528fd4eeb30e978c0b43cfa109768252edaf1f94679dbbc7fe684122c00da34224e2cc1abd2a265af1b267eef1cd34246207946cf7d8e93d2c0f11aa56d8
8673e8f01917b48a5f5476792f759f44ea49d5a5 txgraph: Special-case singletons in chunk index (optimization) (Pieter Wuille)
abdd9d35a34dd9cd589c1b925ab4241d15e6446c txgraph: Skipping end of cluster has no impact (optimization) (Pieter Wuille)
604acc2c289fb187f55439a29dca02d39163df25 txgraph: Reuse discarded chunkindex entries (optimization) (Pieter Wuille)
c734081454d7d7552b1388a27de648bcdd2e4b3a txgraph: Introduce TxGraph::GetWorstMainChunk (feature) (Pieter Wuille)
394dbe21427ee30cca73d2b1e31f18a00a76a1a1 txgraph: Introduce BlockBuilder interface (feature) (Pieter Wuille)
883df3648ee92841e515bb6d8d259cb7054493b1 txgraph: Generalize GetClusterRefs to support subsections (preparation) (Pieter Wuille)
c28a602e007fcc0275bc34f58690820ebe4c9162 txgraph: Introduce TxGraphImpl observer tracking (preparation) (Pieter Wuille)
9095d8ac1c319cdf1b9d44a1fa5993e9a55ccb21 txgraph: Maintain chunk index (preparation) (Pieter Wuille)
87e74e1242ece5fc30e65d4881f24be3a0d02e44 txgraph: abstract out transaction ordering (refactor) (Pieter Wuille)
2614fea17fe3c179ca3a256fadf498ae8cbc03e0 txgraph: Add GetMainStagingDiagrams function (feature) (Pieter Wuille)
Pull request description:
Part of cluster mempool: #30289.
This adds more functionality to the txgraph module, specifically:
* `TxGraph::GetMainStagingDiagrams()`, a function to obtain feerate diagrams for both the main graph and the staged changes to it, including only the clusters that differ between the two.
* `TxGraph::GetBlockBuilder()`, a function to obtain an object which can efficiently iterate the chunks of the (main) graph from high to low chunk feerate, allowing each to be skipped or included.
* `TxGraph::GetWorstMainChunk()`, a function to obtain the last chunk that would be returned by `GetBlockBuilder()`'s returned object, intended for eviction.
ACKs for top commit:
monlovesmango:
reACK 8673e8f01917b48a5f5476792f759f44ea49d5a5
instagibbs:
reACK 8673e8f01917b48a5f5476792f759f44ea49d5a5
glozow:
reACK 8673e8f0191
Tree-SHA512: 5c98c54919c44eb2f9545dfc130e54dfc25b5b54d43cf5ca9bcf46e019b9fd405a572fcd70e71e2a7c5b4b096cfd540a4d09ef1f52ba188504418682f1dfc4af
During init, the test framework will start using rpc after the
mempool was loaded. It will not wait for postInitProcess or
outstanding transactionAddedToMempool notifications, leading to
a possible race, in which listunspent is being called while the
tx is still in Inactive status. Prevent this by processing
outstanding notifications.
There could be a race with outstanding TxAddedToMempool notifications
being applied to the soon-to-be created wallet.
Fixes an intermittent timeout reproducable by adding a sleep to
AddToWallet.
- Create a new function `AddMerkleRootAndCoinbase` that compute the
block's merkle root, insert the coinbase transaction and the merkle
root into the block.
This interface lets one iterate efficiently over the chunks of the main
graph in a TxGraph, in the same order as CompareMainOrder. Each chunk
can be marked as "included" or "skipped" (and in the latter case,
dependent chunks will be skipped).
This is preparation for a next commit which will introduce a class whose
objects hold references to internals in TxGraphImpl, which disallows
modifications to the graph while such objects exist.
415650cea94f5050d7c368fdf9fd9878809957e1 guix: move *-check.py scripts under contrib/guix (fanquake)
Pull request description:
These scripts are not meant for general developer usage. They are for use on the release binaries, which have been compiled in an environment that makes various assumptions in regards to c library, compiler options, hardening options, dependency patching etc.
Anyone is free to run these scripts against self-compiled binaries, but this isn't something we want to modify/generalize the scripts to support.
ACKs for top commit:
laanwj:
Code review ACK 415650cea94f5050d7c368fdf9fd9878809957e1
TheCharlatan:
ACK 415650cea94f5050d7c368fdf9fd9878809957e1
Tree-SHA512: 469cc5da9df014b4a9dc39080f31c3283641691209497cdb7cc5dc4767681a7f4b67425768e81838153ce39a26547ed94cd3be4dc27352d33960765bfac9d0e5
d847e17c9656020de9f378ae82ec89e0bb39ecbd doc: Fix typo (Hennadii Stepanov)
Pull request description:
A translator on Transifex noticed:
> This is the only label which has two dots: ..
> Usually we see the elipsis (…)
This PR addresses this issue.
ACKs for top commit:
janb84:
good find, ACK d847e17c96
Tree-SHA512: cbdc5f283fe30655a923c34813c97406bba85657554984c26acbfed585ce3a652bcb21348d7e0f6fd9c82ca04111c412ff62c7d4b046869d13785b713e69ab6d
5b7ed460c7c181f1fd34a927a09aed36284083cb cmake: Allow `WITH_DBUS` on all Unix-like systems (Hennadii Stepanov)
Pull request description:
This PR makes the `WITH_DBUS` option available on all Unix-like systems, not just Linux, thereby fixing a regression that was overlooked during the migration from Autotools.
Note: Enabling D-Bus support on macOS still makes no sense, since the `Notificator` class uses the User Notification Center regardless:746ab19d5a/src/qt/notificator.cpp (L43-L56)
Fixes https://github.com/bitcoin/bitcoin/issues/32464.
ACKs for top commit:
laanwj:
ACK 5b7ed460c7c181f1fd34a927a09aed36284083cb
theStack:
tACK 5b7ed460c7c181f1fd34a927a09aed36284083cb
Tree-SHA512: 01f5a365a8109c273489e2d46b08d8b64b82251c4814a486561221b55d8a47cb1effc9140dbd6b2ab5f12e2966d0ea0d07a4cbf67d310001ee681b0f5b63f300
3bbdbc0a5e1b409969cedaf249d1d01dea9bcf73 qt, docs: Unify term "clipboard" (Hennadii Stepanov)
Pull request description:
A translator on Transifex noticed:
> The term "system clipboard" appears twice. The term "clipboard" appears 10 times. Perhaps we could standardize on just saying "clipboard"?
This PR addresses this issue.
ACKs for top commit:
davidgumberg:
ACK 3bbdbc0a5e
pablomartin4btc:
ACK 3bbdbc0a5e1b409969cedaf249d1d01dea9bcf73
Tree-SHA512: 61a100f60890d81122a4b8ce3e2cb7d355c7fb643de3196573f7f9107c6f52fa0b3e7a4f743ce2833e8c67b9cdad3568b761d730fef5c9781f5e1c45252888c4
002b792b9a85100d89e47706c29cf1fd355d9727 gui: decouple WalletModel from RPCExecutor (furszy)
Pull request description:
A more comprehensive fix for the issue described in #837.
Since the `WalletModel` class is unavailable when compiling without wallet support
`(-DENABLE_WALLET=0)`, the RPC executor class should not be coupled to it.
This decoupling ensures GUI compatibility with builds that omit wallet support.
This also drops an extra `#ifdef ENABLE_WALLET` block which is always good.
ACKs for top commit:
w0xlt:
Code Review ACK 002b792b9a
pablomartin4btc:
tACK 002b792b9a85100d89e47706c29cf1fd355d9727
BrandonOdiwuor:
tACK 002b792b9a85100d89e47706c29cf1fd355d9727
hebasto:
ACK 002b792b9a85100d89e47706c29cf1fd355d9727, I have reviewed the code and it looks OK.
Tree-SHA512: a8e6b7e9d88dd8e0ff5e2d0de91be2f85fd0559265267d3bf6cae5a37606cf1ab6bc7415d5817a11006008de362f2ca3557ba772b4e1bd9fbef5f564be3b53bb
This change makes the `WITH_DBUS` option available on all Unix-like
systems, not just Linux, thereby fixing a regression that was
overlooked during the migration from Autotools.
Note: Enabling D-Bus support on macOS still makes no sense, since the
`Notificator` class uses the User Notification Center regardless.
ab878a7e741073574336c9c4b1d41c6cf80b0d6a build: simplify *ifaddr handling (fanquake)
Pull request description:
We really just want to skip this when building for Windows. So do that,
and remove the two header checks (we also already use both of these
headers, unguarded, in the !windows part of the codebase).
Squash the two *iffaddrs defines into one, as I haven't seen an
`iffaddrs.h` that implements one, but not the other.
ACKs for top commit:
hebasto:
ACK ab878a7e741073574336c9c4b1d41c6cf80b0d6a. Only addressed my [comment](https://github.com/bitcoin/bitcoin/pull/32446#discussion_r2079994126) and rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/32446#pullrequestreview-2825606189).
TheCharlatan:
ACK ab878a7e741073574336c9c4b1d41c6cf80b0d6a
Tree-SHA512: 7667305df9fef4728526c7217f85b51e739ec63b38e808da51d6ae65cb6f2696afa5ba82e5a72ed4a7a9b79ffa2402640448af4392587253027122eab7618e30
a58cb3b1c12c8cb75a87375c50f94c4605bb805d qa: sanity check mined block have their coinbase timelocked to height (Antoine Poinsot)
8f2078af6a55448c003b3f7f3021955fbb351caa miner: timelock coinbase transactions (Antoine Poinsot)
788aeebf343526760fa8f3ed969ac3713212a5b6 qa: use prev height as nLockTime for coinbase txs created in unit tests (Antoine Poinsot)
c76dbe9b8b6f03b761a0ef97e1b8cd133b934714 qa: timelock coinbase transactions created in fuzz targets (Antoine Poinsot)
9c94069d8b6cf67a24eb03c51230a4f2b2bf2d64 contrib: timelock coinbase transactions in signet miner (Antoine Poinsot)
a5f52cfcc400ad0adb41a78c65b8abb971e0d622 qa: timelock coinbase transactions created in functional tests (Antoine Poinsot)
Pull request description:
The Consensus Cleanup soft fork proposal includes enforcing that coinbase transactions set their
nLockTime field to the block height minus 1, as well as their nSequence such as to not disable the
timelock. If such a fork were to be activated by Bitcoin users, miners need to be ready to produce
compliant blocks at the risk of losing substantial amounts mining would-be invalid blocks. As miners
are unfamously slow to upgrade, it's good to make this change as early as possible.
Although Bitcoin Core's GBT implementation does not provide the `coinbasetxn` field, and mining
pool software crafts the coinbase on its own, updating the Bitcoin Core mining code is a first step
toward convincing pools to update their (often closed source) code. A possible followup is also to
introduce new fields to GBT. In addition, this first step also makes it possible to test future
Consensus Cleanup changes.
The commit making the change also updates a bunch of seemingly-unrelated tests. This is because those tests were asserting error messages based on the txid of transactions involved, and changing the coinbase transaction structure necessarily changes the txid of all tests' transactions.
ACKs for top commit:
Sjors:
Code review ACK a58cb3b1c12c8cb75a87375c50f94c4605bb805d
achow101:
ACK a58cb3b1c12c8cb75a87375c50f94c4605bb805d
TheCharlatan:
Re-ACK a58cb3b1c12c8cb75a87375c50f94c4605bb805d
Tree-SHA512: a2aae009a187eb760d34435f518a895ee76c6b02a667eb030ddf6bd584da6e8eae2737d974dbf81a928d60c07bcb4820f055adc067e18d8819640db0240bb513
1ee698fde2e8652d8eb083749edb8029c003b8db test: refactor: negate signature-s using libsecp256k1 (Sebastian Falbesoner)
Pull request description:
This small PR gets rid of manual mod-n inversion of the ECDSA signature-s part in unit tests (introduced a long time ago in #5256, triggered by https://github.com/bitcoin-core/secp256k1/pull/69) by using secp256k1 instead. The function wasn't available at that time, but was introduced about three years later, see https://github.com/bitcoin-core/secp256k1/pull/408. Note that as the name suggests, `secp256k1_ec_seckey_negate` is meant to be used for secret keys, but it obviously works in general for scalars modulo the group order.
ACKs for top commit:
achow101:
ACK 1ee698fde2e8652d8eb083749edb8029c003b8db
laanwj:
Code review ACK 1ee698fde2e8652d8eb083749edb8029c003b8db
w0xlt:
ACK 1ee698fde2
rkrux:
tACK 1ee698fde2e8652d8eb083749edb8029c003b8db
Tree-SHA512: dc36ea1572b538d11ae34e1871f310a1cda8083ffb753e93e7ee9d56e91ebd8ec78d35758dfb700254720914b734ef7a071eeef71b6239f19e1e2fb289fb5435
