When PR 1022 was merged, an & got dropped from the declarations of the two
"fStall" out-parameters, rendering them unused (and triggering a compiler
warning.) Restore them, restoring the functionality of #1022.
cdfb4c9c6c Fix elements multiple-header-download issue. (Glenn Willen)
Pull request description:
This fixes an issue which causes Elements to download the blockchain headers
multiple times during initial block download.
In particular: each time we receive an INV P2P message with a new block
(about once a minute), we start downloading the headers, again, in parallel
with any existing download(s) in progress.
With this change, after we receive each batch of headers, we check whether
any of the headers in it were new to us. If not (they were all duplicates),
we stop there, and do not ask the peer for another batch. This reduces the
maximum amount of duplication to about 2x, which is not ideal, but a HUGE
improvement.
With luck, this may help with #1013. But it's primarily directed at fixing slow initial block download, which I can't immediately find an open issue for on this repo, but has been a long-standing problem.
ACKs for top commit:
stevenroose:
looks good, utACK cdfb4c9c6c
Tree-SHA512: 693a8f89cae1819292146b5bc676e34cd9045bf3a771d78959d8f704072020d5ccb073d6d5b89786598129fd9fd4f80d2c207a2db29597984fcae774cbfacadb
6c1d0d3ca8 Re-evaluate peg-out descriptor if wallet is reparsed (Jeff Frontz)
Pull request description:
In `sendtomainchain_pak()` a check is made if the associated `descriptor` is null and an attempt to remedy is made -- but the remedy is never applied to `descriptor` (which results in a SEGV later). This code fixes that.
Note: unfortunately it also removes the `const` qualifier. I'm not sure what/if there are conventions for dealing with that (e.g., creating a new `const` of, like, `sanitized_descriptor` for subsequent use in the function?).
Top commit has no ACKs.
Tree-SHA512: 8fb4a28ebd0d76647c1b0ecf98f3e18905c880a9562caa1b02ca66647dcbe00b2a6bd91da7ce6a61bd34bb3096030714417170badccdaeaa4c0a03e8704f8ca1
e72544da90 Mark pegin-related args to createrawtransaction as optional. (Glenn Willen)
Pull request description:
Fixes#1064 .
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors should note which tests cover
modified code. If no tests exist for a region of modified code, new tests
should accompany the change.
* Bug fixes are most welcome when they come with steps to reproduce or an
explanation of the potential issue as well as reasoning for the way the bug
was fixed.
* Features are welcome, but might be rejected due to design or scope issues.
If a feature is based on a lot of dependencies, contributors should first
consider building the system outside of Bitcoin Core, if possible.
* Refactoring changes are only accepted if they are required for a feature or
bug fix or otherwise improve developer experience significantly. For example,
most "code style" refactoring changes require a thorough explanation why they
are useful, what downsides they have and why they *significantly* improve
developer experience or avoid serious programming bugs. Note that code style
is often a subjective matter. Unless they are explicitly mentioned to be
preferred in the [developer notes](/doc/developer-notes.md), stylistic code
changes are usually rejected.
-->
<!--
Bitcoin Core has a thorough review process and even the most trivial change
needs to pass a lot of eyes and requires non-zero or even substantial time
effort to review. There is a huge lack of active reviewers on the project, so
patches often sit for a long time.
-->
Top commit has no ACKs.
Tree-SHA512: a901c4bd65fbd7fceebe39cca9f8e377741e61ef2968bc3c955e26bffb2b371a342b49a09f76696ac45f047d8c410500f5872fb722007fb92b17d0bca97d69dc
This fixes an issue which causes Elements to download the blockchain headers
multiple times during initial block download.
In particular: each time we receive an INV P2P message with a new block
(about once a minute), we start downloading the headers, again, in parallel
with any existing download(s) in progress.
With this change, after we receive each batch of headers, we check whether
any of the headers in it were new to us. If not (they were all duplicates),
we stop there, and do not ask the peer for another batch. This reduces the
maximum amount of duplication to about 2x, which is not ideal, but a HUGE
improvement.
0e1007fa7b minor PSET fix (Andrew Poelstra)
Pull request description:
Noticed because it triggered a compiler warning.
No test -- we will add one in a future "end-to-end issuance via PSET" tutorial script which is out of scope for the 0.21 release.
ACKs for top commit:
achow101:
ACK 0e1007fa7b
Tree-SHA512: c90427fd74c84bdee6940d2148818793e4641e6dd12dd4855a229afb6b725313c6074fc14171c911e5a541e252dd4027e65521c7fe5c62e265fb38076c1d5ca4
Conflicts were mostly obvious, except for those in CreateTransactionInternal.
(This function was moved from wallet.cpp to spend.cpp, and substantially
rewritten, between 0.21 and 22.0.) For those I manually applied the changes
from the diff, which wound up taking a slightly different form.
Also had to update the new test because the `addresses` field of the RPC
output was removed.
a67a2df330 pset: remove one more intermediate-zero check from the blinding logic (Andrew Poelstra)
9eb285c19d test: add test for confidential non-wallet-owned change (Andrew Poelstra)
185d473fe8 walletcreatefundedpsbt: add functional test for blinding edge cases (Andrew Poelstra)
533da12c2c wallet: make sure extra OP_RETURN output is blinded when called from fundraw (Andrew Poelstra)
b09b63bd1b pset: allow input blinding factors to sum to zero, or value to be 0 (Andrew Poelstra)
9afcb83baf wallet: correctly handle blinding of manually-set change addresses (Andrew Poelstra)
9813c3e74a wallet: fix "cannot unblind IsMine output" check in SignPSBT (Andrew Poelstra)
7103471fd5 walletcreatefundedpsbt: signal blinding data correctly to `FundTransaction` (Andrew Poelstra)
Pull request description:
Fixes#1049
Needs backport to 0.21 (and a new rc).
Although there are several bugs here, none affect the functionaries. There is a more thorough summary in the second-to-last commit message.
ACKs for top commit:
achow101:
ACK a67a2df330
Tree-SHA512: 25066c29f080e43cd00c5b33c60a986a8cb5bbf4ca01ceb3b4182c5b8f61979a1d6d946b8f28fa871ec90f44bdb6fc22014b5c11280cf081181db102cd588cdf
0628311b53 chainparams: change magic bytes for liquidv1test (Andrew Poelstra)
Pull request description:
This is basically a Liquid-prod-ish version of regtest. Its default magic should not collide with that for Liquid prod.
ACKs for top commit:
psgreco:
UTACK 0628311b53
Tree-SHA512: 50c387ffeb62d42ffd0b42d0f43335966feb64a50140159c615474b9c864222e2a7510bb3b07aa95f62a977d0e7472d905bc97a06a3ace80aae0a519031c446e
There are a number of special cases (and inconsistencies with wallet.cpp)
in the blinding logic that can prevent blinding from succeeding when various
intermediate values are zero. This fixes two of them.
A proper fix would involve adding new APIs to secp-zkp and then overhauling
all of the wallet blinding code. But for now let's get the tests working.
When the user specifies a change address manually, use the change address
to obtain blinding parameters (either extract the blinding key from the
address or don't blind the change).
The previous behavior would assume that the change address was owned by
the wallet and always generate a blinding key internally. If the user
were to pass a non-wallet-owned change address, the result would be an
output that could not be unblinded by its owner.
A couple issues with this -- one is that it was verifying the asset
commitment against itself, rather than against the explicit asset,
and the other is that the verification logic had an extra `== 0` at
the end which inverted the verification check.
Both pretty embarassing to have not caught in review..
