Commit graph

25014 commits

Author SHA1 Message Date
furszy
05d0576d3c
wallet: create tx, log resulting coin selection info
Useful for understanding what is going on internally
when the software is running. Debug issues, and provide
more accurate feedback to users.

Github-Pull: #28994
Rebased-From: 0c5755761c
2024-01-04 16:21:36 +00:00
Murch
5493ebbe74
wallet: skip BnB when SFFO is active
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>

Github-Pull: #28994
Rebased-From: 5cea25ba79
2024-01-04 16:21:36 +00:00
Martin Zumsande
5097bb3389
rpc: fix getrawtransaction segfault
The crash would happen when querying a mempool transaction with verbosity=2, while pruning.

Github-Pull: #29003
Rebased-From: 494a926d05
2024-01-04 16:21:36 +00:00
Jon Atack
55af112565
p2p: do not make automatic outbound connections to addnode peers
to allocate our limited outbound slots correctly, and to ensure addnode
connections benefit from their intended protections.

Our addnode logic usually connects the addnode peers before the automatic
outbound logic does, but not always, as a connection race can occur.  If an
addnode peer disconnects us and if it was the only one from its network, there
can be a race between reconnecting to it with the addnode thread, and it being
picked as automatic network-specific outbound peer.  Or our internet connection
or router, or the addnode peer, could be temporarily offline, and then return
online during the automatic outbound thread.  Or we could add a new manual peer
using the addnode RPC at that time.

The race can be more apparent when our node doesn't know many peers, or with
networks like cjdns that currently have few bitcoin peers.

When an addnode peer is connected as an automatic outbound peer and is the only
connection we have to a network, it can be protected by our new outbound
eviction logic and persist in the "wrong role".

Examples on mainnet using logging added in the same pull request:

2023-08-12T14:51:05.681743Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic network-specific outbound-full-relay connection
to i2p peer selected for manual (addnode) connection: [geh...odq.b32.i2p]:0

2023-08-13T03:59:28.050853Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic block-relay-only connection to onion peer
selected for manual (addnode) connection: kpg...aid.onion:8333

2023-08-13T16:21:26.979052Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic network-specific outbound-full-relay connection
to cjdns peer selected for manual (addnode) connection: [fcc...8ce]:8333

2023-08-14T20:43:53.401271Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic network-specific outbound-full-relay connection
to cjdns peer selected for manual (addnode) connection: [fc7...59e]:8333

2023-08-15T00:10:01.894147Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic feeler connection to i2p peer selected for
manual (addnode) connection: geh...odq.b32.i2p:8333

Finally, there does not seem to be a reason to make block-relay or short-lived
feeler connections to addnode peers, as the addnode logic will ensure we connect
to them if they are up, within the addnode connection limit.

Fix these issues by checking if the address is an addnode peer in our automatic
outbound connection logic.

Github-Pull: #28895
Rebased-From: cc62716920
2023-11-22 11:53:27 +00:00
Martin Leitner-Ankerl
1488648104
pool: change memusage_test to use int64_t, add allocation check
If alignment of the PoolAllocator would be insufficient, then the test would fail. This also catches the issue with ARM 32bit,
where int64_t is aligned to 8 bytes but void* is aligned to 4 bytes. The test adds a check to ensure the pool has allocated
a minimum number of chunks

Github-Pull: #28913
Rebased-From: d5b4c0b69e
2023-11-22 11:33:12 +00:00
Martin Leitner-Ankerl
bcc183ccce
pool: make sure PoolAllocator uses the correct alignment
This changes the PoolAllocator to default the alignment to the given type. This makes the code simpler, and most importantly
fixes a bug on ARM 32bit that caused OOM: The class CTxOut has a member CAmount which is an int64_t and on ARM 32bit int64_t
are 8 byte aligned which is larger than the pointer alignment of 4 bytes. So for CCoinsMap to be able to use the pool, we
need to use the alignment of the member instead of just alignof(void*).

Github-Pull: #28913
Rebased-From: ce881bf9fc
2023-11-22 11:33:01 +00:00
fanquake
5845331a6c
doc: rewrite explanation for -par=
The negative bound for script threads comes from the machine which
generates the man pages, so may only be correct for that machine. Any
other placeholder value will also be wrong for some machines. Fix this
be removing the value. This also fixes help2man incorrectly bolding the
value, as if it were a paramater.

Closes #28850.

Github-Pull: #28858
Rebased-From: d799ea26ed
2023-11-14 15:46:10 +00:00
Sebastian Falbesoner
e097d4cb53
gui: fix crash on selecting "Mask values" in transaction view
This commits fixes a crash bug that can be caused with the following steps:
- change to the "Transactions" view
- right-click on an arbitrary transaction -> "Show transaction details"
- close the transaction detail window again
- select "Settings" -> "Mask values"

The problem is that the list of opened dialogs, tracked in the member
variable `m_opened_dialogs`, is only ever appended with newly opened
transaction detail dialog pointers, but never removed. This leads to
dangling pointers in the list, and if the "Mask values" menu item is
selected, a crash is caused in the course of trying to close the opened
transaction detail dialogs (see `closeOpenedDialogs()` method). Fix this
by removing the pointer from the list if the corresponding widget is
destroyed.

Github-Pull: https://github.com/bitcoin-core/gui/pull/774
Rebased-From: e26e665f9f
2023-11-01 10:01:05 +00:00
pablomartin4btc
b761a58171
assumeutxo, blockstorage: prevent core dump on invalid hash
Github-Pull: #28698
Rebased-from: 4a5be10b92
2023-10-31 17:07:52 +00:00
Vasil Dimov
d3ebf6e9fc
[test] Test i2p private key constraints
Github-Pull: #28695
Rebased-From: 5cf4d266d9
2023-10-31 17:07:52 +00:00
dergoegge
1f11784aac
[net] Check i2p private key constraints
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

GitHub-Pull: #28695
Rebased-From: cf70a8d565
2023-10-31 17:07:52 +00:00
MarcoFalke
6544ffa01f
bugfix: Mark CNoDestination and PubKeyDestination constructor explicit
This should fix the bug reported in
https://github.com/bitcoin/bitcoin/pull/28246#discussion_r1371640502,
which caused the GUI to not detect the destination type of recipients,
thus picking the wrong change destination type.

Also, add missing lifetimebound attribute to a getter method.

GitHub-Pull: #28728
Rebased-From: 1111475b41
2023-10-31 17:07:52 +00:00
Hennadii Stepanov
1695c4801c
qt: 26.0rc2 translations update 2023-10-31 16:53:32 +00:00
Hennadii Stepanov
74604662f3
qt: 26.0rc1 translations update 2023-10-24 11:01:03 +01:00
Ryan Ofsky
d724bb5291
Merge bitcoin/bitcoin#28609: wallet: Reload watchonly and solvables wallets after migration
4814e4063e test: Check tx metadata is migrated to watchonly (Andrew Chow)
d616d30ea5 wallet: Reload watchonly and solvables wallets after migration (Andrew Chow)
118f2d7d70 wallet: Copy all tx metadata to watchonly wallet (Andrew Chow)
9af87cf348 test: Check that a failed wallet migration is cleaned up (Andrew Chow)

Pull request description:

  Some incomplete/incorrect state as a result of migration can be mitigated/cleaned up by simply restarting the migrated wallets. We already do this for a wallet when it is migrated, but we do not for the new watchonly and solvables wallets that may be created. This PR introduces this behavior, in addition to creating those wallets initially without an attached chain.

  While implementing this, I noticed that not all `CWalletTx` metadata was being copied over to the watchonly wallet and so some data, such as time received, was being lost. This PR fixes this as a side effect of not having a chain attached to the watchonly wallet. A test has also been added.

ACKs for top commit:
  ishaanam:
    light code review ACK 4814e4063e
  ryanofsky:
    Code review ACK 4814e4063e. Just implemented the suggested orderpos, copyfrom, and path set comments since last review
  furszy:
    ACK 4814e406

Tree-SHA512: 0b992430df9f452cb252c2212df8e876613f43564fcd1dc00c6c31fa497adb84dfff6b5ef597590f9b288c5f64cb455f108fcc9b6c9d1fe9eb2c39e7f2c12a89
2023-10-23 17:35:36 -04:00
Andrew Chow
da8e397e4a
Merge bitcoin/bitcoin#28685: coinstats, assumeutxo: fix hash_serialized2 calculation
4bfaad4eca chainparams, assumeutxo: Fix signet txoutset hash (Fabian Jahr)
a503cd0f0b chainparams, assumeutxo: Fix testnet txoutset hash (Fabian Jahr)
f6213929c5 assumeutxo: Check deserialized coins for out of range values (Fabian Jahr)
66865446a7 docs: Add release notes for #28685 (Fabian Jahr)
cb0336817e scripted-diff: Rename hash_serialized_2 to hash_serialized_3 (Fabian Jahr)
351370a1d2 coinstats: Fix hash_serialized2 calculation (Fabian Jahr)

Pull request description:

  Closes #28675

  The last commit demonstrates that theStack's analysis [here](https://github.com/bitcoin/bitcoin/issues/28675#issuecomment-1770389468) seems to be correct. There will be more changes needed for the rest of the test suite but the `feature_assumeutxo.py` with my additional tests pass.

ACKs for top commit:
  achow101:
    ACK 4bfaad4eca
  theStack:
    Code-review ACK 4bfaad4eca
  ryanofsky:
    Code review ACK 4bfaad4eca

Tree-SHA512: 2f6abc92b282f7c5da46391803cf0804d13978d191d541f2509b532c538abccd0a081e46cda23d80d47206a05fa2b5d41b7ab246e6a263db7a7461d6292116ef
2023-10-23 15:16:08 -04:00
Hennadii Stepanov
f09bfab4af
Revert "gui: provide wallet controller context to wallet actions"
This reverts commit 7066e8996d.
2023-10-23 12:14:37 +01:00
fanquake
0046f3dc27
Merge bitcoin/bitcoin#28693: build: Include config/bitcoin-config.h explicitly in util/trace.h
6bdff429ec build: Include `config/bitcoin-config.h` explicitly in `util/trace.h` (Hennadii Stepanov)

Pull request description:

  The `ENABLE_TRACING` macro is expected to be defined in the `config/bitcoin-config.h` header.

  Therefore, the current code is error-prone as it depends on whether the `config/bitcoin-config.h` header was included before or not.

  This bug was noticed while working on CMake [stuff](https://github.com/hebasto/bitcoin/pull/37).

ACKs for top commit:
  fanquake:
    ACK 6bdff429ec

Tree-SHA512: 22c4fdeb51628814050eb99a83db4268a4f3106207eeef918a07214bbc52f2b22490f6b05fcb96216f147afa4197c51102503738131e2583e750b6d195747a49
2023-10-23 11:32:43 +01:00
fanquake
f4e96c29a6
Merge bitcoin/bitcoin#28691: refactor: Remove CBlockFileInfo::SetNull
fac36b94ef refactor: Remove CBlockFileInfo::SetNull (MarcoFalke)

Pull request description:

  Seems better to use C++11 member initializers and then let the compiler figure out how to construct objects of this class.

ACKs for top commit:
  stickies-v:
    ACK fac36b94ef
  pablomartin4btc:
    ACK fac36b94ef
  theStack:
    LGTM ACK fac36b94ef

Tree-SHA512: aee741c8f668f0e5b658fc83f4ebd196b43fead3dd437afdb0a2dafe092ae3d559332b3d9d61985c92e1a59982d8f24942606e6a98598c6ef7ff43697e858725
2023-10-23 10:37:27 +01:00
Fabian Jahr
4bfaad4eca
chainparams, assumeutxo: Fix signet txoutset hash
Review hint: You can use devtools/utxo_snapshot.sh to validate this.

./contrib/devtools/utxo_snapshot.sh 160000 signet-utxo.dat ./src/bitcoin-cli
2023-10-20 22:53:07 +02:00
Fabian Jahr
a503cd0f0b
chainparams, assumeutxo: Fix testnet txoutset hash
Review hint: You can use devtools/utxo_snapshot.sh to validate this.

./contrib/devtools/utxo_snapshot.sh 2500000 testnet-utxo.dat ./src/bitcoin-cli
2023-10-20 22:53:07 +02:00
Fabian Jahr
f6213929c5
assumeutxo: Check deserialized coins for out of range values 2023-10-20 22:53:07 +02:00
Fabian Jahr
cb0336817e
scripted-diff: Rename hash_serialized_2 to hash_serialized_3
-BEGIN VERIFY SCRIPT-
sed -i 's/hash_serialized_2/hash_serialized_3/g' $( git grep -l 'hash_serialized_2' ./src ./contrib ./test )
-END VERIFY SCRIPT-
2023-10-20 22:53:06 +02:00
Fabian Jahr
351370a1d2
coinstats: Fix hash_serialized2 calculation
The legacy serialization was vulnerable to maleation and is fixed by
adopting the same serialization procedure as was already in use for
MuHash.

This also includes necessary test fixes where the hash_serialized2 was
hardcoded as well as correction of the regtest chainparams.

Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
2023-10-20 22:53:05 +02:00
fanquake
abfc8c901d
Merge bitcoin/bitcoin#28692: fuzz: Delete i2p fuzz test
dd4dcbd4cd [fuzz] Delete i2p target (dergoegge)

Pull request description:

  closes #28665

  The target is buggy and doesn't reach basic coverage.

ACKs for top commit:
  maflcko:
    lgtm ACK dd4dcbd4cd
  glozow:
    ACK dd4dcbd4cd, agree it's better to delete this test until somebody wants to write a better one

Tree-SHA512: b6ca6cad1773b1ceb6e5ac0fd501ea615f66507ef811745799deaaa4460f1700d96ae03cf55b740a96ed8cd2283b3d6738cd580ba97f2af619197d6c4414ca21
2023-10-20 15:30:16 +01:00
MarcoFalke
fac36b94ef
refactor: Remove CBlockFileInfo::SetNull 2023-10-20 16:29:02 +02:00
Hennadii Stepanov
6bdff429ec
build: Include config/bitcoin-config.h explicitly in util/trace.h
The `ENABLE_TRACING` macro is expected to be defined in the
`config/bitcoin-config.h` header.

Therefore, the current code is error-prone as it depends on whether the
`config/bitcoin-config.h` header was included before or not.
2023-10-20 14:40:26 +01:00
fanquake
3c856e2fe8
Merge bitcoin/bitcoin#28569: log: Don't log cache rebalancing in absense of a snapshot chainstate
ec84f999f1 log: Don't log cache rebalancing in absense of a snapshot chainstate (Fabian Jahr)

Pull request description:

  I have noticed that this log now is always printed, even if there is no snapshot chainstate present or even was present. I think this is confusing to users that have never even thought about using assumeutxo since in that case the rebalancing is just ensuring the normal environment with one chainstate. So I suggest we don't log in absence of a snapshot chainstate. We could also think about rewording the message instead but I think this is simpler.

ACKs for top commit:
  stickies-v:
    utACK ec84f999f1
  glozow:
    concept ACK ec84f999f1, don't have opinions other than removing confusing log
  theStack:
    utACK ec84f999f1

Tree-SHA512: 30bbfc648e7c788106f78d52e47a3aa1e1874f65d13743643dc50bcf7f450d8330711ff9fdeac361722542da6051533153829c6d49033227ed315e111afc899f
2023-10-20 14:39:34 +01:00
dergoegge
dd4dcbd4cd [fuzz] Delete i2p target 2023-10-20 14:03:34 +01:00
Fabian Jahr
ec84f999f1
log: Don't log cache rebalancing in absense of a snapshot chainstate 2023-10-20 14:53:44 +02:00
Andrew Chow
d616d30ea5 wallet: Reload watchonly and solvables wallets after migration
When migrating, create the watchonly and solvables wallets without a
context. Then unload and reload them after migration completes, as we do
for the actual wallet.

There is also additional handling for a failed reload.
2023-10-19 18:06:43 -04:00
Andrew Chow
118f2d7d70 wallet: Copy all tx metadata to watchonly wallet
When moving a tx to the watchonly wallet during migration, make sure
that all of the CWalletTx data follows it.
2023-10-19 18:06:43 -04:00
Andrew Chow
77f0ceb717
Merge bitcoin/bitcoin#28077: I2P: also sleep after errors in Accept() & destroy the session if we get an unexpected error
5c8e15c451 i2p: destroy the session if we get an unexpected error from the I2P router (Vasil Dimov)
762404a68c i2p: also sleep after errors in Accept() (Vasil Dimov)

Pull request description:

  ### Background

  In the `i2p::sam::Session` class:

  `Listen()` does:
  * if the session is not created yet
    * create the control socket and on it:
    * `HELLO`
    * `SESSION CREATE ID=sessid`
    * leave the control socked opened
  * create a new socket and on it:
  * `HELLO`
  * `STREAM ACCEPT ID=sessid`
  * read reply (`STREAM STATUS`), `Listen()` only succeeds if it contains `RESULT=OK`

  Then a wait starts, for a peer to connect. When connected,

  `Accept()` does:
  * on the socket from `STREAM ACCEPT` from `Listen()`: read the Base64 identification of the connecting peer

  ### Problem

  The I2P router may be in such a state that this happens in a quick succession (many times per second, see https://github.com/bitcoin/bitcoin/issues/22759#issuecomment-1609907115): `Listen()`-succeeds, `Accept()`-fails.

  `Accept()` fails because the I2P router sends something that is not Base64 on the socket: `STREAM STATUS RESULT=I2P_ERROR MESSAGE="Session was closed"`

  We only sleep after failed `Listen()` because the assumption was that if `Accept()` fails then the next `Listen()` will also fail.

  ### Solution

  Avoid filling the log with "Error accepting:" messages and sleep also after a failed `Accept()`.

  ### Extra changes

  * Reset the error waiting time after one successful connection. Otherwise the timer will remain high due to problems that have been solved long time in the past.

  * Increment the wait time less aggressively.

  * Handle the unexpected "Session was closed" message more gracefully (don't log stupid messages like `Cannot decode Base64: "STREAM STATUS...`) and destroy the session right way.

ACKs for top commit:
  achow101:
    ACK 5c8e15c451
  jonatack:
    re-ACK 5c8e15c451

Tree-SHA512: 1d47958c50eeae9eefcb668b8539fd092adead93328e4bf3355267819304b99ab41cbe1b5dbedbc3452c2bc389dc8330c0e27eb5ccb880e33dc46930a1592885
2023-10-19 16:08:06 -04:00
Andrew Chow
0655e9dd92
Merge bitcoin/bitcoin#27071: Handle CJDNS from LookupSubNet()
0e6f6ebc06 net: remove unused CConnman::FindNode(const CSubNet&) (Vasil Dimov)
9482cb780f netbase: possibly change the result of LookupSubNet() to CJDNS (Vasil Dimov)
53afa68026 net: move MaybeFlipIPv6toCJDNS() from net to netbase (Vasil Dimov)
6e308651c4 net: move IsReachable() code to netbase and encapsulate it (Vasil Dimov)
c42ded3d9b fuzz: ConsumeNetAddr(): avoid IPv6 addresses that look like CJDNS (Vasil Dimov)
64d6f77907 net: put CJDNS prefix byte in a constant (Vasil Dimov)

Pull request description:

  `LookupSubNet()` would treat addresses that start with `fc` as IPv6 even if `-cjdnsreachable` is set. This creates the following problems where it is called:

  * `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).

  * `RPCConsole::unbanSelectedNode()`: in the GUI the ban entries go through `CSubNet::ToString()` and back via `LookupSubNet()`. Then it must match whatever is stored in `BanMan`, otherwise it is impossible to unban via the GUI.

  These were uncovered by https://github.com/bitcoin/bitcoin/pull/26859.

  Thus, flip the result of `LookupSubNet()` to CJDNS if the network base address starts with `fc` and `-cjdnsreachable` is set. Since subnetting/masking does not make sense for CJDNS (the address is "random" bytes, like Tor and I2P, there is no hierarchy) treat `fc.../mask` as an invalid `CSubNet`.

  To achieve that, `MaybeFlipIPv6toCJDNS()` has to be moved from `net` to `netbase` and thus also `IsReachable()`. In the process of moving `IsReachable()`, `SetReachable()` and `vfLimited[]` encapsulate those in a class.

ACKs for top commit:
  jonatack:
    Code review ACK 0e6f6ebc06
  achow101:
    ACK 0e6f6ebc06
  mzumsande:
    re-ACK 0e6f6ebc06

Tree-SHA512: 4767a60dc882916de4c8b110ce8de208ff3f58daaa0b560e6547d72e604d07c4157e72cf98b237228310fc05c0a3922f446674492e2ba02e990a272d288bd566
2023-10-19 12:48:39 -04:00
fanquake
9e616baec0
Merge bitcoin/bitcoin#22764: build: Include qt sources for parsing with extract_strings.py
b59b31ae0b build: Drop redundant qt/bitcoin.cpp (Hennadii Stepanov)
d90ad5a42e build: Include qt sources for parsing with extract_strings.py (Hennadii Stepanov)

Pull request description:

  On master (4fc15d1566) some strings are still untranslated.

  This PR fixes this issue.

  To verify:
  1) `./autogen.sh && ./configure && make -C src translate` _before_ applying this change
  2) apply this change
  3) `./autogen.sh && ./configure && make -C src translate` _after_ applying this change

  The result of `git diff src/qt/bitcoinstrings.cpp`:
  ```diff
  --- a/src/qt/bitcoinstrings.cpp
  +++ b/src/qt/bitcoinstrings.cpp
  @@ -126,6 +126,7 @@ QT_TRANSLATE_NOOP("bitcoin-core", ""
   "You need to rebuild the database using -reindex to go back to unpruned "
   "mode.  This will redownload the entire blockchain"),
   QT_TRANSLATE_NOOP("bitcoin-core", "%s is set very high!"),
  +QT_TRANSLATE_NOOP("bitcoin-core", "(press q to shutdown and continue later)"),
   QT_TRANSLATE_NOOP("bitcoin-core", "-maxmempool must be at least %d MB"),
   QT_TRANSLATE_NOOP("bitcoin-core", "A fatal internal error occurred, see debug.log for details"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Cannot resolve -%s address: '%s'"),
  @@ -204,6 +205,8 @@ QT_TRANSLATE_NOOP("bitcoin-core", "SQLiteDatabase: Failed to prepare statement t
   QT_TRANSLATE_NOOP("bitcoin-core", "SQLiteDatabase: Failed to read database verification error: %s"),
   QT_TRANSLATE_NOOP("bitcoin-core", "SQLiteDatabase: Unexpected application id. Expected %u, got %u"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Section [%s] is not recognized."),
  +QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be read"),
  +QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be written"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Signing transaction failed"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Specified -walletdir \"%s\" does not exist"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Specified -walletdir \"%s\" is a relative path"),
  @@ -242,4 +245,5 @@ QT_TRANSLATE_NOOP("bitcoin-core", "User Agent comment (%s) contains unsafe chara
   QT_TRANSLATE_NOOP("bitcoin-core", "Verifying blocks…"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Verifying wallet(s)…"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Wallet needed to be rewritten: restart %s to complete"),
  +QT_TRANSLATE_NOOP("bitcoin-core", "press q to shutdown"),
   };
  ```

ACKs for top commit:
  ryanofsky:
    Code review ACK b59b31ae0b. Being able to use `_()` macro in qt would allow simplifying some code, for example replacing repetitive:
  TheCharlatan:
    ACK b59b31ae0b

Tree-SHA512: 13d9d86b487a1b6e718ae96c198a0a927c881bf33df318412793ec9efba3a7e59cfa836204f73f5b53ff4c99edce778c11bffaa88138b80e37b71e36df6b816f
2023-10-19 13:25:49 +01:00
Andrew Chow
c2d4e40e45
Merge bitcoin/bitcoin#28651: Make miniscript GetWitnessSize accurate for tapscript
b22810887b miniscript: make GetWitnessSize accurate for tapscript (Pieter Wuille)
8be9851408 test: add tests for miniscript GetWitnessSize (Pieter Wuille)
7ed2b2d430 test: remove mutable global contexts in miniscript fuzzer/test (Pieter Wuille)

Pull request description:

  So far, the same algorithm is used to compute an (upper bound on) the maximum witness size for both P2WSH and P2TR miniscript. That's unfortunate, because it means fee estimations for P2TR miniscript will miss out on the generic savings brought by P2TR witnesses (smaller signatures and public keys, specifically).

  Fix this by making the algorithm use script context specification calculations, and add tests for it. Also included is a cleanup for the tests to avoid mutable globals, as I found it hard to reason about what exactly was being tested.

ACKs for top commit:
  achow101:
    ACK b22810887b
  darosior:
    ACK b22810887b

Tree-SHA512: e4bda7376628f3e91cfc74917cefc554ca16eb5f2a0e1adddc33eb8717c4aaa071e56a40f85a2041ae74ec445a7bd0129bba48994c203e0e6e4d25af65954d9e
2023-10-17 18:27:52 -04:00
Hennadii Stepanov
9c30f5ef9d
Merge bitcoin-core/gui#766: Fix coin control input size accounting for taproot spends
00a52e6394 gui: fix coin control input size accounting for taproot spends (Sebastian Falbesoner)

Pull request description:

  If manual coin control is used in the GUI, the input size accounting for P2TR is currently overshooting, as it still assumes P2WPKH (segwitv0) spends which have a larger witness, as ECDSA signatures are longer and the pubkey also has to be provided. Fix that by adding sizes depending on the witness version. Note that the total accounting including outputs is still off and there is some weird logic involved depending on whether SFFO is used, but it's (hopefully) a first step into the right direction.

ACKs for top commit:
  maflcko:
    lgtm ACK 00a52e6394
  furszy:
    utACK 00a52e6394

Tree-SHA512: 9633642f8473247cc3d8e6e0ef502fd515e1dde0e2939d28d6754d0cececedd6a328df22a3d4c85eb2846fd0417cf224b92594613f6e84ada82d2d7d84fc455f
2023-10-17 22:26:25 +01:00
Andrew Chow
fbcf1029a7
Merge bitcoin/bitcoin#28544: wallet: Add TxStateString function for debugging and logging
8a553c9409 wallet: Add TxStateString function for debugging and logging (Ryan Ofsky)

Pull request description:

  I found this useful while debugging silent conflict between #10102 and #27469 recently

ACKs for top commit:
  ishaanam:
    utACK 8a553c9409
  achow101:
    ACK 8a553c9409
  furszy:
    Code ACK 8a553c9

Tree-SHA512: 87965c66bcb59a21e7639878bb567e583a0e624735721ff7ad1104eed6bb9fba60607d0e3de7be3304232b3a55f48bab7039ea9c26b0e81963e59f9acd94f666
2023-10-17 15:28:05 -04:00
fanquake
738ef44abb
Merge bitcoin/bitcoin#28652: assumeutxo: fail early if snapshot block hash doesn't match AssumeUTXO parameters
9620cb4493 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 9620cb4493
  ryanofsky:
    Code review ACK 9620cb4493. This should fix an annoyance and bad UX.
  pablomartin4btc:
    tACK 9620cb4493

Tree-SHA512: f88b865e9d46254858e57c024463f389cd9d8760a7cb30c190aa1723a931e159987dfc2263a733825d700fa612e7416691e4d8aab64058f1aeb0a7fa9233ac9c
2023-10-17 10:20:08 +01:00
Andrew Chow
90f7d8a7f9
Merge bitcoin/bitcoin#28539: lib: add taproot support to libconsensus
ff8e2fc2e2 fuzz: add coverage for `bitcoinconsensus_verify_script_with_spent_outputs` (brunoerg)
c5f2a757d7 docs: add release notes for #28539 (brunoerg)
de54882348 docs: add docs for additional libconsensus functions (Jake Rawsthorne)
70106e0689 docs: link to rust-bitcoinconsensus (Jake Rawsthorne)
fb0db07e41 lib: add Taproot support to libconsensus (Jake Rawsthorne)

Pull request description:

  Grabbed from #21158. Closes #21133.

ACKs for top commit:
  achow101:
    ACK ff8e2fc2e2
  theStack:
    ACK ff8e2fc2e2
  darosior:
    re-ACK ff8e2fc2e2

Tree-SHA512: bf6f500c7e8c9ff6884137c2cd9b4522c586e52848dd639b774b94d998b0516b877498d24f3a6cc7425aedf81d18b0d30c1ccf19e2d527fdfdfa3955ca49b6e7
2023-10-16 12:59:39 -04:00
Sebastian Falbesoner
9620cb4493 assumeutxo: fail early if snapshot block hash doesn't match AssumeUTXO parameters 2023-10-16 17:20:59 +02:00
fanquake
08ea835220
Merge bitcoin/bitcoin#28583: refactor: [tidy] modernize-use-emplace
fa05a726c2 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 fa05a726c2.
  TheCharlatan:
    ACK fa05a726c2

Tree-SHA512: 4408a094f406e7bf6c1468c2b0798f68f4d952a1253cf5b20bdc648ad7eea4a2c070051fed46d66fd37bce2ce6f85962484a1d32826b7ab8c9baba431eaa2765
2023-10-16 15:35:50 +01:00
Hennadii Stepanov
92704535f6
Merge bitcoin-core/gui#765: Fix wallet list hover crash on shutdown
8b6470a906 gui: disable top bar menu actions during shutdown (furszy)
7066e8996d 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 8b6470a906, I've tested each commit separately on macOS Sonoma 14.0 (Apple M1).

Tree-SHA512: 6fbd1bcd6717a8c1633beb9371463ed22422f929cccf9b791ee292c5364134c501e099329cf77a06b74a84c64c1c3d22539199ec49ccd74b3950036316c0dab3
2023-10-16 13:37:35 +01:00
Vasil Dimov
0e6f6ebc06
net: remove unused CConnman::FindNode(const CSubNet&) 2023-10-16 12:59:47 +02:00
Vasil Dimov
9482cb780f
netbase: possibly change the result of LookupSubNet() to CJDNS
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.
2023-10-16 12:57:49 +02:00
fanquake
22fa1f4702
Merge bitcoin/bitcoin#28565: rpc: getaddrmaninfo followups
e6e444c06c refactor: add and use EnsureAnyAddrman in rpc (stratospher)
bf589a50a0 doc: add release notes for #27511 (stratospher)
3931e6abc3 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 e6e444c06c
  theStack:
    Code-review ACK e6e444c06c
  pablomartin4btc:
    tested ACK e6e444c06c

Tree-SHA512: c14090d5c64ff15e92d252578de2437bb2ce2e1e431d6698580241a29190f0a3528ae5b013c0ddb76a9ae538507191295c37cab7fd93469941cadbde44587072
2023-10-16 11:21:45 +02:00
furszy
8b6470a906
gui: disable top bar menu actions during shutdown
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.
2023-10-13 17:40:32 -03:00
furszy
7066e8996d
gui: provide wallet controller context to wallet actions
Addressing potential crashes during shutdown. The most
noticeable one can be triggered by hovering over the
wallet list as the app shuts down.
2023-10-13 17:40:32 -03:00
Pieter Wuille
b22810887b miniscript: make GetWitnessSize accurate for tapscript 2023-10-13 15:28:38 -04:00
Pieter Wuille
8be9851408 test: add tests for miniscript GetWitnessSize 2023-10-13 14:57:03 -04:00