| Age | Commit message (Collapse) | Author |
|---|
|
doesn't match AssumeUTXO parameters
9620cb449374f234f72c1a9e1bae3d4b8c0ff171 assumeutxo: fail early if snapshot block hash doesn't match AssumeUTXO parameters (Sebastian Falbesoner)
Pull request description:
Right now the `loadtxoutset` RPC call treats literally all files with a minimum size of 40 bytes (=size of metadata) as potential valid snapshot candidates and the waiting loop for seeing the metadata block hash in the headers chain is always entered, e.g.:
```
$ ./src/bitcoin-cli loadtxoutset ~/.vimrc
<wait>
bitcoind log:
...
2023-10-15T14:55:45Z [snapshot] waiting to see blockheader 626174207465730a7265626d756e207465730a656c62616e65207861746e7973 in headers chain before snapshot activation
...
```
There is no point in doing any further action though if we already know from the start that the UTXO snapshot loading won't be successful. This PR adds an assumeutxo parameter check immediately after the metadata is read in, so we can fail immediately on a mismatch:
```
$ ./src/bitcoin-cli loadtxoutset ~/.vimrc
error code: -32603
error message:
Unable to load UTXO snapshot, assumeutxo block hash in snapshot metadata not recognized (626174207465730a7265626d756e207465730a656c62616e
65207861746e7973)
```
This way, users who mistakenly try to load files that are not snapshots don't have to wait 10 minutes (=the block header waiting timeout) anymore to get a negative response. If a file is loaded which is a valid snapshot (referencing to an existing block hash), but one which doesn't match the parameters, the feedback is also faster, as we don't have to wait anymore to see the hash in the headers chain before getting an error.
This is also partially fixes #28621.
ACKs for top commit:
maflcko:
lgtm ACK 9620cb449374f234f72c1a9e1bae3d4b8c0ff171
ryanofsky:
Code review ACK 9620cb449374f234f72c1a9e1bae3d4b8c0ff171. This should fix an annoyance and bad UX.
pablomartin4btc:
tACK 9620cb449374f234f72c1a9e1bae3d4b8c0ff171
Tree-SHA512: f88b865e9d46254858e57c024463f389cd9d8760a7cb30c190aa1723a931e159987dfc2263a733825d700fa612e7416691e4d8aab64058f1aeb0a7fa9233ac9c
|
|
ff8e2fc2e2416f6f3b84cdb40db8ac168596b579 fuzz: add coverage for `bitcoinconsensus_verify_script_with_spent_outputs` (brunoerg)
c5f2a757d736f14d27ac5256a9df887cd2f174f1 docs: add release notes for #28539 (brunoerg)
de54882348502d860cf1e504100aa8fb1e52aa88 docs: add docs for additional libconsensus functions (Jake Rawsthorne)
70106e0689546fee497814c63a6a4747e0937b36 docs: link to rust-bitcoinconsensus (Jake Rawsthorne)
fb0db07e414fec3318b3af683167ebef9c82fc84 lib: add Taproot support to libconsensus (Jake Rawsthorne)
Pull request description:
Grabbed from #21158. Closes #21133.
ACKs for top commit:
achow101:
ACK ff8e2fc2e2416f6f3b84cdb40db8ac168596b579
theStack:
ACK ff8e2fc2e2416f6f3b84cdb40db8ac168596b579
darosior:
re-ACK ff8e2fc2e2416f6f3b84cdb40db8ac168596b579
Tree-SHA512: bf6f500c7e8c9ff6884137c2cd9b4522c586e52848dd639b774b94d998b0516b877498d24f3a6cc7425aedf81d18b0d30c1ccf19e2d527fdfdfa3955ca49b6e7
|
|
parameters
|
|
fa05a726c225dc65dee79367bb67f099ae4f99e6 tidy: modernize-use-emplace (MarcoFalke)
Pull request description:
Constructing a temporary unnamed object only to copy or move it into a container seems both verbose in code and a strict performance penalty.
Fix both issues via the `modernize-use-emplace` tidy check.
ACKs for top commit:
Sjors:
re-utACK fa05a726c2
hebasto:
ACK fa05a726c225dc65dee79367bb67f099ae4f99e6.
TheCharlatan:
ACK fa05a726c225dc65dee79367bb67f099ae4f99e6
Tree-SHA512: 4408a094f406e7bf6c1468c2b0798f68f4d952a1253cf5b20bdc648ad7eea4a2c070051fed46d66fd37bce2ce6f85962484a1d32826b7ab8c9baba431eaa2765
|
|
8b6470a90652fcffc45b8d7998af7c8ad6251332 gui: disable top bar menu actions during shutdown (furszy)
7066e8996d0ac090535cc97cdcb54a219986460f gui: provide wallet controller context to wallet actions (furszy)
Pull request description:
Small follow-up to #751.
Fixes another crash cause during shutdown. Which occurs when the user hovers over the wallets list.
Future Note:
This surely happen in other places as well, we should re-work the way we connect signals. Register
lambas without any precaution can leave dangling pointers.
ACKs for top commit:
hebasto:
ACK 8b6470a90652fcffc45b8d7998af7c8ad6251332, I've tested each commit separately on macOS Sonoma 14.0 (Apple M1).
Tree-SHA512: 6fbd1bcd6717a8c1633beb9371463ed22422f929cccf9b791ee292c5364134c501e099329cf77a06b74a84c64c1c3d22539199ec49ccd74b3950036316c0dab3
|
|
|
|
All callers of `LookupSubNet()` need the result to be of CJDNS type if
`-cjdnsreachable` is set and the address begins with `fc`:
* `NetWhitelistPermissions::TryParse()`: otherwise `-whitelist=` fails
to white list CJDNS addresses: when a CJDNS peer connects to us, it
will be matched against IPv6 `fc...` subnet and the match will never
succeed.
* `BanMapFromJson()`: CJDNS bans are stored as just IPv6 addresses in
`banlist.json`. Upon reading from disk they have to be converted back
to CJDNS, otherwise, after restart, a ban entry like (`fc00::1`, IPv6)
would not match a peer (`fc00::1`, CJDNS).
* `setban()` (in `rpc/net.cpp`): otherwise `setban fc.../mask add` would
add an IPv6 entry to BanMan. Subnetting does not make sense for CJDNS
addresses, thus treat `fc.../mask` as invalid `CSubNet`. The result of
`LookupHost()` has to be converted for the case of banning a single
host.
* `InitHTTPAllowList()`: not necessary since before this change
`-rpcallowip=fc...` would match IPv6 subnets against IPv6 peers even
if they started with `fc`. But because it is necessary for the above,
`HTTPRequest::GetPeer()` also has to be adjusted to return CJDNS peer,
so that now CJDNS peers are matched against CJDNS subnets.
|
|
e6e444c06cbf09380f9924dff3d21c1be15d1753 refactor: add and use EnsureAnyAddrman in rpc (stratospher)
bf589a50a0d6a7b94f1ba1ddf24a1497fd35ad44 doc: add release notes for #27511 (stratospher)
3931e6abc39b8aee1472028dbf76eeb10708d2b4 rpc: `getaddrmaninfo` followups (stratospher)
Pull request description:
- make `getaddrmaninfo` RPC public since it's not for development purposes only and regular users might find it useful. [#26988 (comment)](https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1738371584)
- add missing `all_networks` key to RPC help. [#27511 (comment)](https://github.com/bitcoin/bitcoin/pull/27511#discussion_r1335084087)
- fix clang format spacing
- add and use `EnsureAddrman` in RPC code. [#27511 (comment)](https://github.com/bitcoin/bitcoin/pull/27511#discussion_r1331501491)
ACKs for top commit:
0xB10C:
Code Review re-ACK e6e444c06cbf09380f9924dff3d21c1be15d1753
theStack:
Code-review ACK e6e444c06cbf09380f9924dff3d21c1be15d1753
pablomartin4btc:
tested ACK e6e444c06cbf09380f9924dff3d21c1be15d1753
Tree-SHA512: c14090d5c64ff15e92d252578de2437bb2ce2e1e431d6698580241a29190f0a3528ae5b013c0ddb76a9ae538507191295c37cab7fd93469941cadbde44587072
|
|
Opening the top bar menu when the app is being destroyed
freezes the GUI shutdown process for no reason. No menu
action can be executed.
Note:
This behavior is consistent with how the tray icon menu
is cleared too.
|
|
Addressing potential crashes during shutdown. The most
noticeable one can be triggered by hovering over the
wallet list as the app shuts down.
|
|
|
|
|
|
|
|
Co-authored-by: Antonie Poinsot <darosior@protonmail.com>
|
|
Co-authored-by: Bruno Garcia <brunoely.gc@gmail.com>
|
|
SnapshotMetadata constructor, fix test, add test
fafde92f84fb7c245bc3c1cd946a32c891861e5e test: Check snapshot file with wrong number of coins (MarcoFalke)
faa90f6e7b6b8c531e1ae142a5c2f568b48502a9 refactor: Remove unused nchaintx from SnapshotMetadata constructor (MarcoFalke)
Pull request description:
See commit messages
ACKs for top commit:
Sjors:
utACK fafde92f84fb7c245bc3c1cd946a32c891861e5e
theStack:
ACK fafde92f84fb7c245bc3c1cd946a32c891861e5e
Tree-SHA512: 9ed2720b50d1c0938f30543ba143e1a4c6af3a0ff166f8b3eb452e1d99ddee6e3443a4c99f77efe94b8c3eb2feff984bf5259807ee8085e1e0e1e0d1de98227e
|
|
|
|
|
|
Also, remove wrong nChainTx comment and cast.
|
|
keys when inferring
74c77825e5ab68bfa575dad86444506c43ef6c06 test: Unit test for inferring scripts with hybrid and uncompressed keys (Andrew Chow)
f895f97014ac5fac46d27725c1ea7decf7ff76d4 test: Scripts with hybrid pubkeys are migrated to watchonly (Andrew Chow)
37b9b734770e855b9beff3b5085125f1420dd072 descriptors: Move InferScript's pubkey validity checks to InferPubkey (Andrew Chow)
b7485f11ab3a0f1860b261f222362af3301e0781 descriptors: Check result of InferPubkey (Andrew Chow)
Pull request description:
`InferDescriptor` was not always checking that the pubkey it was placing into the descriptor was an allowed pubkey. For example, given a P2WPKH script that uses an uncompressed pubkey, it would produce a `wpkh()` with the uncompressed key. Additionally, the hybrid key check was only being done for `pk()` scripts, where it should've been done for all scripts.
This PR moves the key checking into `InferPubkey`. If the key is not valid for the context, then `nullptr` is returned and the inferring will fall through to the defaults of either `raw()` or `addr()`.
This also resolves an issue with migrating legacy wallets that contain hybrid pubkeys as such watchonly scripts will become `raw()` or `addr()` and go to the watchonly wallet. Note that a legacy wallet cannot sign for hybrid pubkeys. A test has been added for the migration case.
Also added unit tests for `InferDescriptor` itself as the edge cases with that function are not covered by the descriptor roundtrip test.
ACKs for top commit:
furszy:
ACK 74c77825
Sjors:
utACK 74c77825e5ab68bfa575dad86444506c43ef6c06
Tree-SHA512: ed5f63e42a2e46120245a6b0288b90d2a6912860814c6c08fe393332add1cb364dc5eca72f16980352143570aef0c07bf1a91acd294099463bd028b6ce2fe40c
|
|
|
|
and check disk space periodically
ed52e71176fc97c6ed01e3eebd85acdec54b4448 Periodically check disk space to avoid corruption (Aurèle Oulès)
7fe537f7a48675b1d25542bee6f390d665547580 Implement CCoinsViewErrorCatcher::HaveCoin (Aurèle Oulès)
Pull request description:
Attempt to fix #26112.
As suggested by sipa in https://github.com/bitcoin/bitcoin/issues/26112#issuecomment-1249683401:
> CCoinsViewErrorCatcher, the wrapper class used around CCoinsViewDB that's supposed to detect these problems and forcefully exit the application, has an override for GetCoins. But in CheckTxInputs, HaveInputs is first invoked, which on its turn calls HaveCoin. HaveCoin is implemented in CCoinsViewDB, but not in CCoinsViewErrorCatcher, and thus the disk read exception escapes.
> A solution may be to just add an override for HaveCoin in CCoinsViewErrorCatcher.
I implemented `CCoinsViewErrorCatcher::HaveCoin` and also added a periodic disk space check that shutdowns the node if there is not enough space left on disk, the minimum here is 50MB.
For reviewers, it's possible to saturate disk space to test the PR by creating large files with `fallocate -l 50G test.bin`
ACKs for top commit:
achow101:
ACK ed52e71176fc97c6ed01e3eebd85acdec54b4448
w0xlt:
Code Review ACK https://github.com/bitcoin/bitcoin/pull/26331/commits/ed52e71176fc97c6ed01e3eebd85acdec54b4448
sipa:
utACK ed52e71176fc97c6ed01e3eebd85acdec54b4448
Tree-SHA512: 456aa7b996023df42b4fbb5158ee429d9abf7374b7b1ec129b21aea1188ad19be8da4ae8e0edd90b85b7a3042b8e44e17d3742e33808a4234d5ddbe9bcef1b78
|
|
|
|
|
|
InferPubkey can return a nullptr, so check it's result before continuing
with creating the inferred descriptor.
|
|
bd71f03df75d2c17926b6d575ffa886daa334e3a doc: update example pulls in release-process.md (fanquake)
b2ede22395ae8ce371433c9611929374dd98908a headerssync: update params for 26.x (fanquake)
f12f92b813cd8c29904f36f8ed7ed74649886897 kernel: update m_assumed_* chain params for 26.x (fanquake)
a8c2e5e556daf2a8c6b013110c802768b3f4b30e kernel: update chainTxData for 26.x (fanquake)
a9d070a6f89d855aec5fbe6efe679feef86a21f3 kernel: update nMinimumChainWork & defaultAssumeValid for 26.x (fanquake)
Pull request description:
Update chainparams pre `26.x` branch off.
Note: Remember that some variance is expected in the m_assumed_* sizes.
Closes #28572.
ACKs for top commit:
Sjors:
re-ACK bd71f03df75d2c17926b6d575ffa886daa334e3a
ajtowns:
reACK bd71f03df75d2c17926b6d575ffa886daa334e3a
darosior:
re-ACK bd71f03df75d2c17926b6d575ffa886daa334e3a for assumevalid and minchainwork params.
Tree-SHA512: c586b82711477bfc1fd9ac4c9c1130ccde29f75d652492c66ef5730f4a49b7da822a3ad2d9090468ea0c9f0e77d00c25f6a800600c81878d8141ce2ffb3724af
|
|
5f504065544133a47da5a7a240675c23eceb0799 Adjust Gradle properties (Hennadii Stepanov)
Pull request description:
On the master branch @ d2b8c5e1234cdaff84bd1f60aea598d219cdac5e, building the `apk` target fails:
```
$ make -C src/qt apk
...
> Task :compileDebugJavaWithJavac FAILED
/home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/QtActivityDelegate.java:690: error: cannot find symbol
Display display = (Build.VERSION.SDK_INT < Build.VERSION_CODES.R)
^
symbol: variable R
location: class VERSION_CODES
/home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/QtActivityDelegate.java:692: error: cannot find symbol
: m_activity.getDisplay();
^
symbol: method getDisplay()
location: variable m_activity of type Activity
/home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/QtActivityDelegate.java:833: error: cannot find symbol
float refreshRate = (Build.VERSION.SDK_INT < Build.VERSION_CODES.R)
^
symbol: variable R
location: class VERSION_CODES
/home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/QtActivityDelegate.java:835: error: cannot find symbol
: m_activity.getDisplay().getRefreshRate();
^
symbol: method getDisplay()
location: variable m_activity of type Activity
/home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/QtLayout.java:95: error: cannot find symbol
Display display = (Build.VERSION.SDK_INT < Build.VERSION_CODES.R)
^
symbol: variable R
location: class VERSION_CODES
/home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/QtLayout.java:97: error: cannot find symbol
: ((Activity)getContext()).getDisplay();
^
symbol: method getDisplay()
location: class Activity
/home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/ExtractStyle.java:418: error: cannot find symbol
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q)
^
symbol: variable Q
location: class VERSION_CODES
/home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/ExtractStyle.java:421: error: cannot find symbol
numStates = stateList.getStateCount();
^
symbol: method getStateCount()
location: variable stateList of type StateListDrawable
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
8 errors
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':compileDebugJavaWithJavac'.
> Compilation failed; see the compiler error output for details.
* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.
* Get more help at https://help.gradle.org
Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.6.1/userguide/command_line_interface.html#sec:command_line_warnings
BUILD FAILED in 827ms
...
```
Fixing it by updating the Gradle tool's properties.
ACKs for top commit:
fanquake:
ACK 5f504065544133a47da5a7a240675c23eceb0799 - seems fine.
Tree-SHA512: 52e59fe1c69841370ce2eb670f3618182bf2843582074af4895b8ecb6e5f70dc3fe4eecbffa212efaa534b423ced5b75020f6f09917b52f452121c1e55fbcaac
|
|
The diff is produced by running `make -C src translate`.
|
|
This change fixes the `apk` target build after bumping Qt version from
5.15.5 to 5.15.10.
|
|
|
|
To avoid recursive calls in shared_ptr's destructor that could lead to a
stack overflow.
|
|
We make the Satisfier a base in which to store the common methods
between the Tapscript and P2WSH satisfier, and from which they both
inherit.
A field is added to SignatureData to be able to satisfy pkh() under
Tapscript context (to get the pubkey hash preimage) without wallet data.
For instance in `finalizepsbt` RPC. See also the next commits for a
functional test that exercises this.
|
|
We'll need the Miniscript satisfier for Tapscript too.
|
|
|
|
64-hex-characters public keys are valid in Miniscript key expressions
within a Tapscript context.
Keys under a Tapscript context always serialize as 32-bytes x-only
public keys (and that's what get hashed by OP_HASH160 on the stack too).
|
|
In order to exacerbate a mistake in the stack size tracking logic,
sometimes pad the witness to make the script execute at the brink of the
stack size limit. This way if the stack size is underestimated for a
script it would immediately fail `VerifyScript`.
|
|
|
|
Under Tapscript, due to the lifting of some standardness and consensus
limits, scripts can now run into the maximum stack size during
execution. Any Miniscript that may hit the limit on any of its spending
paths must be marked as unsafe.
Co-Authored-By: Pieter Wuille <pieter@wuille.net>
|
|
We introduce another global that dictates the script context under which
to operate when running the target.
For miniscript_script, just consume another byte to set the context.
This should only affect existing seeds to the extent they contain a
CHECKMULTISIG. However it would not invalidate them entirely as they may
contain a NUMEQUAL or a CHECKSIGADD, and this still exercises a bit of
the parser.
For miniscript_string, reduce the string size by one byte and use the
last byte to determine the context. This is the change that i think
would invalidate the lowest number of existing seeds.
For miniscript_stable, we don't want to invalidate any seed. Instead of
creating a new miniscript_stable_tapscript, simply run the target once
for P2WSH and once for Tapscript (with the same seed).
For miniscript_smart, consume one byte before generating a pseudo-random
node to set the context. We have less regard for seed stability for this
target anyways.
|
|
Adapt the test data and the parsing context to support x-only keys.
Adapt the Test() helper to test existing cases under both Tapscript and
P2WSH context, asserting what needs to be valid or not in each.
Finally, add more cases that exercise the logic that was added in the
previous commits (multi_a, different resource checks and keys
serialization under Tapscript, different properties for 'd:' fragment,
..).
|
|
We'll need to get a compressed key out of an x-only one in other places.
Avoid duplicating the code.
|
|
|
|
Under Tapscript, there is:
- No limit on the number of OPs
- No limit on the script size, it's implicitly limited by the maximum
(standard) transaction size.
- No standardness limit on the number of stack items, it's limited by
the consensus MAX_STACK_SIZE. This requires tracking the maximum stack
size at all times during script execution, which will be tackled in
its own commit.
In order to avoid any Miniscript that would not be spendable by a
standard transaction because of the size of the witness, we limit the
script size under Tapscript to the maximum standard transaction size
minus the maximum possible witness and Taproot control block sizes. Note
this is a conservative limit but it still allows for scripts more than a
hundred times larger than under P2WSH.
|
|
|
|
|
|
In Tapscript MINIMALIF is a consensus rule, so we can rely on the fact
that the `DUP IF [X] ENDIF` will always put an exact 1 on the stack upon
satisfaction.
|
|
It is the equivalent of multi() but for Tapscript, using CHECKSIGADD
instead of CHECKMULTISIG.
It shares the same properties as multi() but for 'n', since a threshold
multi_a() may have an empty vector as the top element of its
satisfaction. It could also have the 'o' property when it only has a
single key, but in this case a 'pk()' is always preferable anyways.
|
|
CHECKMULTISIG is disabled for Tapscript. Instead, we'll introduce
a multi_a() fragment with the same semantic as multi().
|
|
Some checks will be different depending on the script context (for
instance the maximum script size).
|
|
We are going to introduce Tapscript support in Miniscript, for which
some of Miniscript rules and properties change (new or modified
fragments, different typing rules, different resources consumption, ..).
|
