0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-26 13:01:49 -05:00
Commit graph

27346 commits

Author SHA1 Message Date
MarcoFalke
6d8eecd33a refactor: Avoid implicit-integer-sign-change in createTransaction 2024-03-21 19:32:12 +01:00
TheCharlatan
824f47294a
node: Use log levels in noui_ThreadSafeMessageBox 2024-03-21 16:41:16 +01:00
TheCharlatan
ddc7872c08
node: Make translations of fatal errors consistent
The extra `bilingual_str` argument of the fatal error notifications and
`node::AbortNode()` is often unused and when used usually contains the
same string as the message argument. It also seems to be confusing,
since it is not consistently used for errors requiring user action. For
example some assumeutxo fatal errors require the user to do something,
but are not translated.

So simplify the fatal error and abort node interfaces by only passing a
translated string. This slightly changes the fatal errors displayed to
the user.

Also de-duplicate the abort error log since it is repeated in noui.cpp.
2024-03-21 16:40:22 +01:00
Luke Dashjr
4f273ab436 Change Luke Dashjr seed to dashjr-list-of-p2p-nodes.us
To avoid issues with DNS blacklisting, I've setup a separate domain for my DNS seed.
2024-03-21 15:26:20 +00:00
willcl-ark
69d6fd676e
cli: improve bitcoin-cli error when not connected
Adds a string suggestion `bitcoin-cli -help` as an additional source of
information.
2024-03-21 10:29:43 +00:00
Ava Chow
bf1b6383db
Merge bitcoin/bitcoin#29671: index: avoid "failed to commit" errors on initialization
f65b0f6401 index: Move last_locator_write_time and logging to end of threadsync loop (Fabian Jahr)

Pull request description:

  In the index sync thread, when initializing an index for the first time, stop callng BaseIndex::Commit when m_best_block_index is null, to avoid a spurious "failed to commit" error from that function. This error started happening in commit 7878f97bf1 from https://github.com/bitcoin/bitcoin/pull/25494 and was reported by pstratem in https://github.com/bitcoin/bitcoin/pull/26903 with an alternate fix.

ACKs for top commit:
  achow101:
    ACK f65b0f6401
  ryanofsky:
    Code review ACK f65b0f6401. Just moved log "Syncing" log line since last commit to avoid having to call now() twice.
  furszy:
    ACK f65b0f6401
  TheCharlatan:
    ACK f65b0f6401

Tree-SHA512: afa8f05786318d36346d167ff53ea0b3bc8abdb0ad04465d199dc3eb91e9f837369e24fcb7e24b5757b02d698ec504e61da6ac365eaf006c874fc07a424a7e20
2024-03-20 16:38:32 -04:00
Martin Zumsande
32c80413fd bench: add benchmark for checkblockindex 2024-03-20 16:30:04 -04:00
Ava Chow
460ae1bf67 wallet, rpc: Add createwalletdescriptor RPC 2024-03-20 16:15:43 -04:00
Ava Chow
8e1a475062 wallet: Be able to retrieve single key from descriptors
Adds CWallet::GetKey which retrieves a single key from the descriptors
stored in the wallet.
2024-03-20 16:15:43 -04:00
Ava Chow
85b1fb19dd wallet: Add GetActiveHDPubKeys to retrieve xpubs from active descriptors 2024-03-20 16:15:43 -04:00
Andrew Chow
73926f2d31 wallet, descspkm: Refactor wallet descriptor generation to standalone func 2024-03-20 16:15:43 -04:00
Andrew Chow
54e74f46ea wallet: Refactor function for single DescSPKM setup
We will need access to a function that sets up a singular
DescriptorSPKM, so refactor this out of the multiple DescriptorSPKM
setup function.
2024-03-20 16:15:43 -04:00
Ava Chow
5febe28c9e wallet, rpc: Add gethdkeys RPC
gethdkeys retrieves all HD keys stored in the wallet's descriptors.
2024-03-20 16:15:43 -04:00
ishaanam
5952292133 wallet, rpc: show mempool conflicts in gettransaction result 2024-03-20 15:05:37 -04:00
ishaanam
54e07ee22f wallet: track mempool conflicts
Behavior changes are:
- if a tx has a mempool conflict, the wallet will not attempt to
  rebroadcast it
- if a txo is spent by a mempool-conflicted tx, that txo is no
  longer considered spent
2024-03-20 15:05:34 -04:00
Ava Chow
b50554babd
Merge bitcoin/bitcoin#29370: assumeutxo: Get rid of faked nTx and nChainTx values
9d9a7458a2 assumeutxo: Remove BLOCK_ASSUMED_VALID flag (Ryan Ofsky)
ef174e9ed2 test: assumeutxo snapshot block CheckBlockIndex crash test (Ryan Ofsky)
0391458d76 test: assumeutxo stale block CheckBlockIndex crash test (Ryan Ofsky)
ef29c8b662 assumeutxo: Get rid of faked nTx and nChainTx values (Ryan Ofsky)
9b97d5bbf9 doc: Improve comments describing setBlockIndexCandidates checks (Ryan Ofsky)
0fd915ee6b validation: Check GuessVerificationProgress is not called with disconnected block (Ryan Ofsky)
63e8fc912c ci: add getchaintxstats ubsan suppressions (Ryan Ofsky)
f252e687ec assumeutxo test: Add RPC test for fake nTx and nChainTx values (Ryan Ofsky)

Pull request description:

  The `PopulateAndValidateSnapshot` function introduced in f6e2da5fb7 from #19806 has been setting fake `nTx` and `nChainTx` values that can show up in RPC results (https://github.com/bitcoin/bitcoin/issues/29328) and make `CBlockIndex` state hard to reason about, because it is difficult to know whether the values are real or fake.

  Revert to previous behavior of setting `nTx` and `nChainTx` to 0 when the values are unknown, instead of faking them. Also drop no-longer needed `BLOCK_ASSUMED_VALID` flag.

  Dropping the faked values also fixes assert failures in the `CheckBlockIndex` `(pindex->nChainTx == pindex->nTx + prev_chain_tx)` check that could happen previously if forked or out-of-order blocks before the snapshot got submitted while the snapshot was being validated. The PR includes two commits adding tests for these failures and describing them in detail.

  Compatibility note: This change could cause new `-checkblockindex` failures if a snapshot was loaded by a previous version of Bitcoin Core and not fully validated, because fake `nTx` values will have been saved to the block index. It would be pretty easy to avoid these failures by adding some compatibility code to `LoadBlockIndex` and changing `nTx` values from 1 to 0 when they are fake (when `(pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TRANSACTIONS`), but a little simpler not to worry about being compatible in this case.

ACKs for top commit:
  Sjors:
    re-ACK 9d9a7458a2
  achow101:
    ACK 9d9a7458a2
  mzumsande:
    Tested ACK 9d9a7458a2
  maflcko:
    ACK 9d9a7458a2 🎯

Tree-SHA512: b1e1e2731ec36be30d5f5914042517219378fc31486674030c29d9c7488ed83fb60ba7095600f469dc32f0d8ba79c49ff7706303006507654e1762f26ee416e0
2024-03-20 12:56:49 -04:00
Ava Chow
69ddee6f39
Merge bitcoin/bitcoin#27039: blockstorage: do not flush block to disk if it is already there
dfcef536d0 blockstorage: do not flush block to disk if it is already there (Matthew Zipkin)

Pull request description:

  Closes https://github.com/bitcoin/bitcoin/issues/2039

  When reindexing from flat-file block storage there is no need to write anything back to disk, since the block data is already there. This PR skips flushing to disk those blocks that already have a known position in the datastore. Skipping this means that users can write-protect the `blk` files on disk which may be useful for security or even safely sharing that data between multiple bitcoind instances.

  `FindBlockPos()` may also flush the undo data file, but again this is skipped if the corresponding block position is known, like during the initial stage of a reindex when block data is being indexed. Once the block index is complete the validation mechanism will call `ConnectBlock()` which will save undo data at that time.

  The call stack looks like this:

  ```
  init()
  ThreadImport() <-- process fReindex flag
  LoadExternalBlockFile()
  AcceptBlock()
  SaveBlockToDisk()
  FindBlockPos()
  FlushBlockFile() <-- unnecessary if block is already on disk
  ```

  A larger refactor of this part of the code was started by mzumsande here:  https://github.com/mzumsande/bitcoin/tree/202207_refactor_findblockpos including this fix, reviewers can let me know if the changes should be combined.

ACKs for top commit:
  sipa:
    utACK dfcef536d0
  mzumsande:
    re-ACK dfcef536d0
  achow101:
    ACK dfcef536d0
  furszy:
    Rebase diff ACK dfcef53.

Tree-SHA512: 385c5ac1288b325135398d0ddd3ab788fa98cc0ca19bd2474c74039f2ce70d5088c1d1c9d4dd10aefcbd4c757767ec5805d07ba8cee9289a66f96e6f9eaa5279
2024-03-20 12:41:33 -04:00
Ava Chow
0b96a1925e
Merge bitcoin/bitcoin#28955: index: block filters sync, reduce disk read operations by caching last header
99afb9d15a refactor: init, simplify index shutdown code (furszy)
0faafb57f8 index: decrease ThreadSync cs_main contention (furszy)
f1469eb454 index: cache last block filter header (furszy)
a6756ecdb2 index: blockfilter, decouple header lookup into its own function (furszy)
331f044e3b index: blockfilter, decouple Write into its own function (furszy)
bcbd7eb8d4 bench: basic block filter index initial sync (furszy)

Pull request description:

  Work decoupled from #26966 per request.

  The aim is to remove an unnecessary disk read operation that currently takes place with every new arriving block (or scanned block during background sync). Instead of reading the last filter header from disk merely to access its hash for constructing the next filter, this work caches it, occupying just 32 more bytes in memory.

  Also, reduces `cs_main` lock contention during the index initial sync process. And, simplifies the indexes initialization and shutdown procedure.

  Testing Note:
  To compare the changes, added a pretty basic benchmark in the second commit. Alternatively, could also test the changes by timing the block filter sync from scratch on any network; start the node with `-blockfilterindex` and monitor the logs until the syncing process finish.

  Local Benchmark Results:

  *Master (c252a0fc0f):
  |               ns/op |                op/s |    err% |     total | benchmark
  |--------------------:|--------------------:|--------:|----------:|:----------
  |      132,042,516.60 |                7.57 |    0.3% |      7.79 | `BlockFilterIndexSync`

  *PR (43a212cfdac6c64e82b601c664443d022f191520):
  |               ns/op |                op/s |    err% |     total | benchmark
  |--------------------:|--------------------:|--------:|----------:|:----------
  |      126,915,841.60 |                7.88 |    0.6% |      7.51 | `BlockFilterIndexSync`

ACKs for top commit:
  Sjors:
    re-ACK 99afb9d15a
  achow101:
    ACK 99afb9d15a
  TheCharlatan:
    Re-ACK 99afb9d15a
  andrewtoth:
    ACK 99afb9d15a

Tree-SHA512: 927daadd68f4ee1ca781a89519539b895f5185a76ebaf525fbc246ea8dcf40d44a82def00ac34b188640802844b312270067f1b33e65a2479e06be9169c616de
2024-03-20 12:30:38 -04:00
Fabian Jahr
f65b0f6401
index: Move last_locator_write_time and logging to end of threadsync loop
This avoids having commit print a needless error message during init.

Co-authored-by: furszy <mfurszy@protonmail.com>
2024-03-19 18:20:39 +01:00
fanquake
479ecc0515
Merge bitcoin/bitcoin#29192: Weaken serfloat tests
6e873df347 serfloat: improve/simplify tests (Pieter Wuille)
b45f1f5658 serfloat: do not test encode(bits)=bits anymore (Pieter Wuille)

Pull request description:

  Closes #28941.

  Our current tests for serfloat verify two distinct properties:
  1. Whether they roundtrip `double`->`uint64_t`->`double` (excluding NaN values) on all systems.
  2. Whether on systems with a typical floating point unit that encoding matches the hardware representation, as before v22.0, we would dump the hardware representation directly to disk and we wanted to retain compatibility with that.

  #28941 seems to show that the second property doesn't always hold, but just for "subnormal" numbers (below $2^{-1021}$). Since we don't care about encoding these numbers, we could exclude such subnormal numbers from the hardware-identical representation test, but this PR goes further and just drops the second property entirely, as I don't think we care about edge-case compatibility with pre-v22.0 code for fee_estimates.dat (the only place it is used).

ACKs for top commit:
  glozow:
    ACK 6e873df347
  fanquake:
    ACK 6e873df347 - It's not as much of a priority, but I think we could still backport this.

Tree-SHA512: e18ceee0753a7ee7e999fdfa10b014dc5bb67b6ef79522a0f8c76b889adcfa785772fc26ed7559bcb5a09a9938e243bb54eedd9549bc59080a2c8090155e2267
2024-03-19 17:09:07 +00:00
0xb10c
6205466512
rpc: "addpeeraddress tried" return error on failure
When trying to add an address to the IP address manager tried table,
it's first added to the new table and then moved to the tried table.
Previously, adding a conflicting address to the address manager's
tried table with test-only `addpeeraddress tried=true` RPC would
return `{ "success": true }`. However, the address would not be added
to the tried table, but would remain in the new table. This caused,
e.g., issue 28964.

This is fixed by returning `{ "success": false, "error":
"failed-adding-to-tried" }` for failed tried table additions. Since
the address remaining in the new table can't be removed (the address
manager interface does not support removing addresses at the moment
and adding this seems to be a bigger effort), an error message is
returned. This indicates to a user why the RPC failed and allows
accounting for the extra address in the new table.

Also:
To check the number of addresses in each addrman table,
the addrman checks were re-run and the log output of this check
was asserted. Ideally, logs shouldn't be used as an interface
in automated tests. To avoid asserting the logs, use the getaddrmaninfo
and getrawaddrman RPCs (which weren't implemented when the test was added).
Removing the "getnodeaddress" calls would also remove the addrman checks
from the test, which could reduce the test coverage. To avoid this,
these are kept.
2024-03-19 17:38:33 +01:00
fanquake
0f89e86516
Merge bitcoin/bitcoin#29667: fuzz: actually test garbage >64b in p2p transport test
626f8e398e fuzz: actually test garbage >64b in p2p transport test (Pieter Wuille)

Pull request description:

  This fixes an oversight from #28196: in the `p2p_transport_bidirectional_v2` fuzz test, when the desired garbage length is over 64 bytes, the code would actually use garbage length 0. Fix this.

ACKs for top commit:
  instagibbs:
    ACK 626f8e398e
  brunoerg:
    crACK 626f8e398e

Tree-SHA512: f6346367adb10464b6c9d20aef43625531d2a4d8110887ad03214b8c1907b83560f2dd5b5415e2180a40b4cd276d51881b32b60c740471b5c6bb218aa19848d8
2024-03-19 12:20:33 +00:00
glozow
5d045c31a5
Merge bitcoin/bitcoin#28950: RPC: Add maxfeerate and maxburnamount args to submitpackage
38f70ba6ac RPC: Add maxfeerate and maxburnamount args to submitpackage (Greg Sanders)

Pull request description:

  Resolves https://github.com/bitcoin/bitcoin/issues/28949

  I couldn't manage to do it very cleanly outside of (sub)package evaluation itself, since it would change the current interface very heavily. Instead I threaded through the max fee argument and used that directly via ATMPArgs. From that perspective, this is somewhat a reversion from https://github.com/bitcoin/bitcoin/pull/19339. In a post-cluster mempool world, these checks could be consolidated to right after the given (ancestor) package is linearized/chunked, by just checking the feerate of the top chunk and rejecting the submission entirely if the top chunk is too high.

  The implication here is that subpackages can be submitted to the mempool prior to hitting this new fee-based error condition.

ACKs for top commit:
  ismaelsadeeq:
    Re-ACK 38f70ba6ac 👍🏾
  glozow:
    ACK 38f70ba6ac with some non-blocking nits
  murchandamus:
    LGTM, code review ACK 38f70ba6ac

Tree-SHA512: 38212aa9de25730944cee58b0806a3d37097e42719af8dd7de91ce86bb5d9770b6f7c37354bf418bd8ba571c52947da1dcdbb968bf429dd1dbdf8715315af18f
2024-03-18 18:24:06 +00:00
fanquake
80f8b92f4f
remove libbitcoinconsensus
This was deprecated in v27.0, for removal in v28.0.
See discussion in PR #29189.
2024-03-18 16:59:39 +00:00
Ryan Ofsky
9d9a7458a2 assumeutxo: Remove BLOCK_ASSUMED_VALID flag
Flag adds complexity and is not currently used for anything.
2024-03-18 11:28:40 -05:00
Ryan Ofsky
ef29c8b662 assumeutxo: Get rid of faked nTx and nChainTx values
The `PopulateAndValidateSnapshot` function introduced in
f6e2da5fb7 from #19806 has been setting fake
`nTx` and `nChainTx` values that can show up in RPC results (see #29328) and
make `CBlockIndex` state hard to reason about, because it is difficult to know
whether the values are real or fake.

Revert to previous behavior of setting `nTx` and `nChainTx` to 0 when the
values are unknown, instead of faking them.

This commit fixes at least two assert failures in the (pindex->nChainTx ==
pindex->nTx + prev_chain_tx) check that would happen previously. Tests for
these failures are added separately in the next two commits.

Compatibility note: This change could result in -checkblockindex failures if a
snapshot was loaded by a previous version of Bitcoin Core and not fully
validated, because fake nTx values will have been saved to the block index. It
would be pretty easy to avoid these failures by adding some compatibility code
to `LoadBlockIndex` and changing `nTx` values from 1 to 0 when they are fake
(when `(pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TRANSACTIONS`), but a
little simpler not to worry about being compatible in this case.
2024-03-18 11:28:40 -05:00
Ryan Ofsky
9b97d5bbf9 doc: Improve comments describing setBlockIndexCandidates checks
The checks are changing slightly in the next commit, so try to explains the
ones that exist to avoid confusion
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1499519079)
2024-03-18 11:28:40 -05:00
Ryan Ofsky
0fd915ee6b validation: Check GuessVerificationProgress is not called with disconnected block
Use Assume macro as suggested https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1479427801
2024-03-18 11:28:40 -05:00
Ava Chow
66632e5c24 wallet: Add IsActiveScriptPubKeyMan
Given a ScriptPubKeyMan, it's useful to ask the wallet whether it is
currently active.
2024-03-18 11:53:23 -04:00
Ava Chow
fa6a259985 desc spkm: Add functions to retrieve specific private keys 2024-03-18 11:53:20 -04:00
fanquake
aba9024c0c
Merge bitcoin/bitcoin#29659: ci: Bump TIDY_LLVM_V
636c9862cf ci: Bump `TIDY_LLVM_V` (Hennadii Stepanov)

Pull request description:

  This PR switches to the latest [IWYU 0.22](https://github.com/include-what-you-use/include-what-you-use/releases/tag/0.22), which is compatible with Clang 18.

ACKs for top commit:
  fanquake:
    ACK 636c9862cf

Tree-SHA512: 78ce89244c5e487dd1be8b4bd2ca6f06d19b04b78289ebc21985110574053545dcce5eb622edf2bede2cf7bb58360170e976d30a4484a127d34dd17b1c604e9c
2024-03-18 15:10:31 +00:00
MarcoFalke
fa72dcbfa5
refactor: FormatISO8601* without gmtime* 2024-03-18 16:01:24 +01:00
MarcoFalke
fa2c486afc
Revert "time: add runtime sanity check"
This reverts commit 3c2e16be22.
2024-03-18 16:01:08 +01:00
Greg Sanders
7295986778 Unit tests for CalculateFeerateDiagramsForRBF 2024-03-18 10:32:00 -04:00
Greg Sanders
b767e6bd47 test: unit test for ImprovesFeerateDiagram 2024-03-18 10:32:00 -04:00
Greg Sanders
7e89b659e1 Add fuzz test for FeeFrac 2024-03-18 10:32:00 -04:00
Greg Sanders
4d6528a3d6 fuzz: fuzz diagram creation and comparison
Co-authored-by: Suhas Daftuar <sdaftuar@chaincode.com>
Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
2024-03-18 10:32:00 -04:00
Greg Sanders
e9c5aeb11d test: Add tests for CompareFeerateDiagram and CheckConflictTopology 2024-03-18 10:32:00 -04:00
Greg Sanders
588a98dccc fuzz: Add fuzz target for ImprovesFeerateDiagram
Co-authored-by: Suhas Daftuar <sdaftuar@chaincode.com>
2024-03-18 10:32:00 -04:00
Greg Sanders
2079b80854 Implement ImprovesFeerateDiagram
This new function takes the populated sets of
direct and all conflicts computed in the current
mempool, assuming the replacements are a single
chunk, and computes a diagram check.

The diagram check only works against cluster
sizes of 2 or less, and fails if it encounters
a different topology.

Co-authored-by: Suhas Daftuar <sdaftuar@chaincode.com>
2024-03-18 10:32:00 -04:00
Greg Sanders
66d966dcfa Add FeeFrac unit tests
Co-authored-by: Suhas Daftuar <sdaftuar@chaincode.com>
2024-03-18 10:32:00 -04:00
Greg Sanders
ce8e22542e Add FeeFrac utils
Co-authored-by: Suhas Daftuar <sdaftuar@chaincode.com>
Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
2024-03-18 10:32:00 -04:00
Pieter Wuille
626f8e398e fuzz: actually test garbage >64b in p2p transport test 2024-03-17 11:35:01 -04:00
ishaanam
d64922b590 wallet refactor: use CWalletTx member functions to determine tx state 2024-03-15 18:03:42 -04:00
Hennadii Stepanov
636c9862cf
ci: Bump TIDY_LLVM_V
This change switches to the latest IWYU 0.22, which is compatible with
Clang 18.
2024-03-15 13:34:05 +00:00
Luke Dashjr
d1ed09a764 Bugfix: GUI: Help messages already have a trailing newline, so don't add an extra one 2024-03-15 00:38:33 +00:00
Luke Dashjr
c3e632b441 Bugfix: bitcoin-cli: Check length of peer.transport_protocol_type 2024-03-15 00:32:49 +00:00
ishaanam
ffe5ff1fb6 scripted-diff: wallet: s/TxStateConflicted/TxStateBlockConflicted
-BEGIN VERIFY SCRIPT-
sed -i 's/TxStateConflicted/TxStateBlockConflicted/g' src/wallet/wallet.cpp src/wallet/interfaces.cpp src/wallet/transaction.h src/wallet/transaction.cpp
sed -i 's/isConflicted/isBlockConflicted/g' src/wallet/transaction.h src/wallet/wallet.cpp
-END VERIFY SCRIPT-
2024-03-14 17:38:39 -04:00
MarcoFalke
fa5844f06d
Remove unused g++-10 workaround
This reverts d4999d40b9
2024-03-14 15:53:43 +01:00
Matthew Zipkin
c70e4fc9a3
netbase: remove unnecessary log message
see https://github.com/bitcoin/bitcoin/pull/27375#issuecomment-1994498888
2024-03-13 14:09:50 -04:00