Commit Graph

39325 Commits

Author SHA1 Message Date
Sergi Delgado Segura
34b9ef443b net/rpc: Makes CConnman::GetAddedNodeInfo able to return only non-connected address on request
`CConnman::GetAddedNodeInfo` is used both to get a list of addresses to manually connect to
in `CConnman::ThreadOpenAddedConnections`, and to report about manually added connections in
`getaddednodeinfo`. In both cases, all addresses added to `m_added_nodes` are returned, however
the nodes we are already connected to are only relevant to the latter, in the former they are
actively discarded.

Parametrizes `CConnman::GetAddedNodeInfo` so we can ask for only addresses we are not connected to,
to avoid passing useless information around.
2023-10-30 11:39:21 -04:00
Sergi Delgado Segura
94e8882d82 rpc: Prevents adding the same ip more than once when formatted differently
Currently it is possible to add the same node twice when formatting IPs in
different, yet equivalent, manner. This applies to both ipv4 and ipv6, e.g:

127.0.0.1 = 127.1 | [::1] = [0:0:0:0:0:0:0:1]

`addnode` will accept both and display both as connected (given they translate to
the same IP). This will not result in multiple connections to the same node, but
will report redundant info when querying `getaddednodeinfo` and populate `m_added_nodes`
with redundant data.

This can be avoided performing comparing the contents of `m_added_addr` and the address
to be added as `CServices` instead of as strings.
2023-10-30 11:39:19 -04:00
Sergi Delgado Segura
2574b7e177 net/rpc: Check all resolved addresses in ConnectNode rather than just one
The current `addnode` rpc command has some edge cases in where it is possible to
connect to the same node twice by combining ip and address requests. This can happen under two situations:

The two commands are run one right after each other, in which case they will be processed
under the same loop in `CConnman::ThreadOpenAddedConnections` without refreshing `vInfo`, so both
will go trough. An example of this would be:

```
bitcoin-cli addnode "localhost:port" add

```

A node is added by IP using `addnode "add"` while the other is added by name using
`addnode "onetry"` with an address that resolves to multiple IPs. In this case, we currently
only check one of the resolved IPs (picked at random), instead of all the resolved ones, meaning
this will only probabilistically fail/succeed. An example of this would be:

```
bitcoin-cli addnode "127.0.0.1:port" add
[...]
bitcoin-cli addnode "localhost:port" onetry
```

Both cases can be fixed by iterating over all resolved addresses in `CConnman::ConnectNode` instead
of picking one at random
2023-10-30 11:21:57 -04:00
fanquake
7d6c646cc7
Merge bitcoin/bitcoin#28348: build: Bump g++ minimum supported version to 10
fa5423b5b5 refactor: Remove unused gcc-9 workaround in txrequest (MarcoFalke)
fa918d397d Always enable -Wsuggest-override (MarcoFalke)
faea58eee4 Bump g++ minimum supported version to 10 (MarcoFalke)

Pull request description:

  All supported operating systems ship with g++ 10 (or later), so bumping the minimum should not cause any issues. The bump allows to drop some now-unused workarounds.

  For reference:
  * https://packages.debian.org/bullseye/g++ (`g++-10`)
  * https://packages.ubuntu.com/focal/g++-10
  * FreeBSD 12/13 ships with g++ 12
  * CentOS-like 9 ships with g++ 11
  * OpenSuse Tumbleweed ships with g++ 13 https://software.opensuse.org/package/gcc13-c++ (No idea about OpenSuse Leap)

  This is for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month.

ACKs for top commit:
  fanquake:
    ACK fa5423b5b5

Tree-SHA512: 6f0697ae4c0f578873591b7872bf158aba3af17f171c3556b593a70ec379bf94c7a9dd7697e8e79173edd4ac3c81a376e0cbbc0cfabde1a1cfe5f9b5eaea6831
2023-10-30 15:55:39 +01:00
MarcoFalke
fa5423b5b5
refactor: Remove unused gcc-9 workaround in txrequest 2023-10-30 15:18:40 +01:00
MarcoFalke
fa918d397d
Always enable -Wsuggest-override 2023-10-30 15:18:22 +01:00
MarcoFalke
faea58eee4
Bump g++ minimum supported version to 10
Also, enable -Werror=maybe-uninitialized in
ci/test/00_setup_env_native_qt5.sh
2023-10-30 15:12:26 +01:00
MarcoFalke
fa56067a8f
refactor: Fix bugprone-string-constructor warning 2023-10-30 14:59:17 +01:00
fanquake
6391644b66
Merge bitcoin/bitcoin#28737: doc: Fix bugprone-lambda-function-name errors
faa769db5a Fix bugprone-lambda-function-name errors (MarcoFalke)

Pull request description:

  Inside a lambda, `__func__` will evaluate to something like `"operator()"`. Fix this by either removing it, or by using the real name.

  https://clang.llvm.org/extra/clang-tidy/checks/bugprone/lambda-function-name.html

ACKs for top commit:
  TheCharlatan:
    ACK faa769db5a
  darosior:
    utACK faa769db5a

Tree-SHA512: 0b562bd4ebd7f46ca3ebabeee67851ad30bd522fa57e5010e833b163664e51f5df645ff9ca35d22c3479fb27d9267d4e5d0d417d42729bf3ccf80d7944970e4e
2023-10-30 14:54:11 +01:00
fanquake
ec5116ae14
Merge bitcoin/bitcoin#28695: net: Sanity check private keys received from SAM proxy
5cf4d266d9 [test] Test i2p private key constraints (Vasil Dimov)
cf70a8d565 [net] Check i2p private key constraints (dergoegge)

Pull request description:

  Not sanity checking can lead to crashes or worse:

  ```
  ==1715589==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6140000055c2 at pc 0x5622ed66e7ad bp 0x7ffee547a2c0 sp 0x7ffee547a2b8
  READ of size 2 at 0x6140000055c2 thread T0 (b-test)
      #0 0x5622ed66e7ac in memcpy include/bits/string_fortified.h:29:10
      #1 0x5622ed66e7ac in i2p::sam::Session::MyDestination() const src/i2p.cpp:362:5
      #2 0x5622ed662e46 in i2p::sam::Session::CreateIfNotCreatedAlready() src/i2p.cpp:414:40
      #3 0x5622ed6619f2 in i2p::sam::Session::Listen(i2p::Connection&) src/i2p.cpp:143:9
  ```

ACKs for top commit:
  maflcko:
    code lgtm ACK 5cf4d266d9
  stickies-v:
    re-ACK 5cf4d266d9
  vasild:
    ACK 5cf4d266d9

Tree-SHA512: 3de3bd396538fa619de67957b9c8a58011ab911f0f51097c387e730c13908278b7322aa3357051fb245a20b15bef34b0e9fadcb1eff8ad751139d2aa634c78ad
2023-10-30 14:44:40 +01:00
fanquake
a3670b2273
Merge bitcoin/bitcoin#28753: test: Remove feature_txindex_compatibility.py
897d6dd42b Remove feature_txindex_compatibility.py in V27 (Brandon Odiwuor)

Pull request description:

  Fixes https://github.com/bitcoin/bitcoin/issues/28421, see [#28195 (comment)](fa8685597e (r1311494362))

  Remove feature_txindex_compatibility.py in V27, follow up to https://github.com/bitcoin/bitcoin/pull/28195 being merged which is included in v26

ACKs for top commit:
  maflcko:
    lgtm ACK 897d6dd42b
  theStack:
    ACK 897d6dd42b
  stickies-v:
    ACK 897d6dd42b

Tree-SHA512: 53102d39f6fdbdcf1bb13b6feb2f446b0e9e8e3fe294c0e6fe37e7731713fb9fd5b048e19b6edf80579f5edbcf762b51d56d57bdcda67ec3527706891dc3572b
2023-10-30 14:09:15 +01:00
MarcoFalke
99990194ce
Remove WithParams serialization helper 2023-10-30 13:54:52 +01:00
Vasil Dimov
5cf4d266d9 [test] Test i2p private key constraints 2023-10-30 11:41:11 +00:00
Brandon Odiwuor
897d6dd42b Remove feature_txindex_compatibility.py in V27 2023-10-30 13:35:12 +03:00
fanquake
feae4e0438
Merge bitcoin/bitcoin#28698: assumeutxo, blockstorage: Prevent core dump on invalid hash
811067ca1c test: add coverage for snapshot chainstate not matching AssumeUTXO parameters (pablomartin4btc)
4a5be10b92 assumeutxo, blockstorage: prevent core dump on invalid hash (pablomartin4btc)

Pull request description:

  While reviewing #27596 (ran `loadtxoutset` in `mainnet` before `m_assumeutxo_data` is empty as [currently](434495a8c1/src/kernel/chainparams.cpp (L175-L177)) in master  - back to 1b1d711), got a `core dumped`, so it seems there's a potential issue if new releases ever remove snapshot details or a semi-experienced user performs a `loadtxoutset` on a different "customised" binary version (not sure if this is a real use case).

  ```
  2023-10-18T17:42:52Z [init] Using obfuscation key for /tmp/.test_utxo_2/blocks/index: 0000000000000000
  node/blockstorage.cpp:390 LoadBlockIndex: Assertion `GetParams().AssumeutxoForBlockhash(*snapshot_blockhash)' failed.
  Aborted (core dumped)
  ```

  <details>
  <summary>This is also happening before IBD is completed (<code>background validation</code> still being performed as it can be seen in rpc <code>getchainstates</code>)</summary>

  ```
  /src/bitcoin-cli -datadir=${AU_DATADIR} getchainstates
  {
    "headers": 813097,
    "chainstates": [
      {
        "blocks": 368249,
        "bestblockhash": "00000000000000000b7a08224a1cb00d337100ba7a46c03d04b2c2d8964efc37",
        "difficulty": 52278304845.59168,
        "verificationprogress": 0.086288278873286,
        "coins_db_cache_bytes": 7969177,
        "coins_tip_cache_bytes": 14908338995,
        "validated": true
      },
      {
        "blocks": 813097,
        "bestblockhash": "0000000000000000000270c9fdce7b17db64cca91f90106964b58e33a4d91089",
        "difficulty": 61030681983175.59,
        "verificationprogress": 0.999997140098457,
        "coins_db_cache_bytes": 419430,
        "coins_tip_cache_bytes": 784649420,
        "snapshot_blockhash": "00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054",
        "validated": false
      }
    ]
  }
  ```
  </details>

  <details>
  <summary>Steps to reproduce the core dump error and its output:</summary>

  1. Perform a `loadtxoutset` in `mainnet` on compiled `bitcoind` adding the block hash from Sjors's [commit](24deb2022b).
  2. Once step 1 finishes, remove the added code from step 1 and compile again or just compile `master` without any changes on top.
  3. Run `bitcoind`, soon it'll crash with:

  ```
  2023-10-18T17:42:52Z [init] init message: Loading block index…
  2023-10-18T17:42:52Z [init] Assuming ancestors of block 00000000000000000001a0a448d6cf2546b06801389cc030b2b18c6491266815 have valid signatures.
  2023-10-18T17:42:52Z [init] Setting nMinimumChainWork=000000000000000000000000000000000000000052b2559353df4117b7348b64
  2023-10-18T17:42:52Z [init] Prune configured to target 3000 MiB on disk for block and undo files.
  2023-10-18T17:42:52Z [init] [snapshot] detected active snapshot chainstate (/tmp/.test_utxo_2/chainstate_snapshot) - loading
  2023-10-18T17:42:52Z [init] [snapshot] switching active chainstate to Chainstate [snapshot] @ height -1 (null)
  2023-10-18T17:42:52Z [init] Opening LevelDB in /tmp/.test_utxo_2/blocks/index
  2023-10-18T17:42:52Z [init] Opened LevelDB successfully
  2023-10-18T17:42:52Z [init] Using obfuscation key for /tmp/.test_utxo_2/blocks/index: 0000000000000000
  node/blockstorage.cpp:390 LoadBlockIndex: Assertion `GetParams().AssumeutxoForBlockhash(*snapshot_blockhash)' failed.
  Aborted (core dumped)
  ```
  </details>

  <details>
  <summary>After original change, error message output:</summary>

  ```
  2023-10-20T15:49:12Z [init] init message: Loading block index…
  2023-10-20T15:49:12Z [init] Assuming ancestors of block 00000000000000000001a0a448d6cf2546b06801389cc030b2b18c6491266815 have valid signatures.
  2023-10-20T15:49:12Z [init] Setting nMinimumChainWork=000000000000000000000000000000000000000052b2559353df4117b7348b64
  2023-10-20T15:49:12Z [init] Prune configured to target 3000 MiB on disk for block and undo files.
  2023-10-20T15:49:12Z [init] [snapshot] detected active snapshot chainstate (/tmp/.test_utxo_2/chainstate_snapshot) - loading
  2023-10-20T15:49:12Z [init] [snapshot] switching active chainstate to Chainstate [snapshot] @ height -1 (null)
  2023-10-20T15:49:12Z [init] Opening LevelDB in /tmp/.test_utxo_2/blocks/index
  2023-10-20T15:49:12Z [init] Opened LevelDB successfully
  2023-10-20T15:49:12Z [init] Using obfuscation key for /tmp/.test_utxo_2/blocks/index: 0000000000000000
  2023-10-20T15:49:13Z [init] *** Assumeutxo data not found for the given blockhash '00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054'.
  2023-10-20T15:49:13Z [init] Error: Assumeutxo data not found for the given blockhash '00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054'.
  Error: Assumeutxo data not found for the given blockhash '00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054'.
  2023-10-20T15:49:13Z [init] Shutdown requested. Exiting.
  2023-10-20T15:49:13Z [init] Shutdown: In progress...
  2023-10-20T15:49:13Z [scheduler] scheduler thread exit
  2023-10-20T15:49:13Z [shutoff] Flushed fee estimates to fee_estimates.dat.
  2023-10-20T15:49:13Z [shutoff] Shutdown: done
  ```
  </details>

  <details>
  <summary>Alternative on error handling using <code>return error()</code> instead of <code>return FatalError()</code> used in this PR, which produces a different output and perhaps confusing:</summary>

  ```
  2023-10-20T21:45:58Z [init] Using obfuscation key for /tmp/.test_utxo_2/blocks/index: 0000000000000000
  2023-10-20T21:45:59Z [init] ERROR: Assumeutxo data not found for the given blockhash '00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054'.
  2023-10-20T21:45:59Z [init] : Error loading block database.
  Please restart with -reindex or -reindex-chainstate to recover.
  : Error loading block database.
  Please restart with -reindex or -reindex-chainstate to recover.
  2023-10-20T21:45:59Z [init] Aborted block database rebuild. Exiting.
  2023-10-20T21:45:59Z [init] Shutdown: In progress...
  2023-10-20T21:45:59Z [scheduler] scheduler thread exit
  2023-10-20T21:45:59Z [shutoff] Flushed fee estimates to fee_estimates.dat.
  2023-10-20T21:45:59Z [shutoff] Shutdown: done
  ```

  </details>

  <details>
  <summary>Current state (including ryanofsky <a href="https://github.com/bitcoin/bitcoin/pull/28698#discussion_r1368635965">suggestion</a>), after code change, error message output:</summary>

  ```
  2023-10-25T02:29:57Z [init] Using obfuscation key for /home/pablo/.test_utxo_2/regtest/blocks/index: 0000000000000000
  2023-10-25T02:29:57Z [init] *** Assumeutxo data not found for the given blockhash 'f09b5835f3f8b39481f2af3257bbc2e82845552d4d2d6d31cf520fc24263ed5b'.
  2023-10-25T02:29:57Z [init] Error: A fatal internal error occurred, see debug.log for details
  Error: A fatal internal error occurred, see debug.log for details
  2023-10-25T02:29:57Z [init] Shutdown requested. Exiting.
  2023-10-25T02:29:57Z [init] Shutdown: In progress...
  2023-10-25T02:29:57Z [scheduler] scheduler thread exit
  2023-10-25T02:29:57Z [shutoff] Flushed fee estimates to fee_estimates.dat.
  2023-10-25T02:29:57Z [shutoff] Shutdown: done
  ```

  </details>

ACKs for top commit:
  naumenkogs:
    ACK 811067ca1c
  theStack:
    ACK 811067ca1c
  ryanofsky:
    Code review ACK 811067ca1c.

Tree-SHA512: cfc137b0a4f638b99fd7dac2c35cc729ef71ae1166a2a8960a91055ec90841cb33aed589834012cfe0e157937e2a76a88d1020ea1df2bc98e1114eb1fc8eaae4
2023-10-29 10:22:10 +01:00
fanquake
f028470654
Merge bitcoin/bitcoin#28727: test: replace random_bytes with random.randbytes
fe3ac3700d test: replace random_bytes with randbytes #28720 (ns-xvrn)

Pull request description:

  With Python upgraded to 3.9 replaced the `random_bytes` function in util of functional tests and replaced it's usage with `random.randbytes`.

  Closes #28720.

ACKs for top commit:
  maflcko:
    lgtm ACK fe3ac3700d
  BrandonOdiwuor:
    ACK fe3ac3700d
  stickies-v:
    ACK fe3ac3700d, thanks for picking this up
  kristapsk:
    utACK fe3ac3700d

Tree-SHA512: f65a75e73ebd840c2936eb133d42bccd552f25b717c8ca25c18d06e0593e12f292389cfcc0a0b0759004b67a46ea0c8ac237973ef90f246139778230be1e64e1
2023-10-29 10:14:59 +01:00
fanquake
42b0d5f59b
Merge bitcoin/bitcoin#28740: refactor: Add LIFETIMEBOUND to all (w)txid getters
faec889f93 refactor: Add LIFETIMEBOUND to all (w)txid getters (MarcoFalke)

Pull request description:

  Currently some getters return a reference, some don't. Fix this by returning a reference everywhere. Also, add `LIFETIMEBOUND` to all. Then, use the compiler warnings to create copies only where needed.

  Also, fix iwyu includes while touching the includes.

ACKs for top commit:
  dergoegge:
    Code review ACK faec889f93
  stickies-v:
    ACK faec889f93
  pablomartin4btc:
    cr ACK faec889f93

Tree-SHA512: 0c2a151f39d0e007b4d33b0b85ad578cc220f3e9dd94890e812b3181c3901545b039325707731cc39a5e89557f59c1154c6320525f78f5de95f119a514d2d23f
2023-10-29 10:10:53 +01:00
MarcoFalke
faec889f93
refactor: Add LIFETIMEBOUND to all (w)txid getters
Then, use the compiler warnings to create copies only where needed.

Also, fix iwyu includes while touching the includes.
2023-10-27 13:01:42 +02:00
Brandon Odiwuor
3c208cc05e Add offline signing tutorial 2023-10-27 12:32:48 +03:00
Andrew Chow
e789b30b25
Merge bitcoin/bitcoin#27116: doc: clarify that LOCK() internally checks whether the mutex is held
91d0888921 sync: unpublish LocksHeld() which is used only in sync.cpp (Vasil Dimov)
3df37e0c78 doc: clarify that LOCK() does AssertLockNotHeld() internally (Vasil Dimov)

Pull request description:

  Constructs like

  ```cpp
  AssertLockNotHeld(m);
  LOCK(m);
  ```

  are equivalent to (almost, modulo some logging differences, see below)

  ```cpp
  LOCK(m);
  ```

  for non-recursive mutexes, so it is ok to omit `AssertLockNotHeld()` in such cases. Requests to do the former keep coming during review process. `developer-notes.md` explicitly states "Combine annotations in function declarations with run-time asserts in function definitions", but that seems to be too strong or unclear. `LOCK()` is also a run-time assert in this case.

  Also remove `LocksHeld()` from the public interface in `sync.h` since it is only used in `sync.cpp`.

ACKs for top commit:
  achow101:
    ACK 91d0888921
  hebasto:
    ACK 91d0888921, I have reviewed the code and it looks OK.

Tree-SHA512: c4b7ef2c0bfeb28d1c4f55f497810f629873137e02f5a92137c02cb1ff603ac76473dcd2171e594491494a5cb87b8c0c803e06b86f190d4acb231791e28e802d
2023-10-26 15:02:13 -04:00
Andrew Chow
b72cb7801b
Merge bitcoin/bitcoin#28283: doc: clarify cookie generation in JSON-RPC-interface.md
bdb2e8d4ae Update JSON-RPC-interface.md (iamcarlos94)

Pull request description:

  clarifying when the .cookie file is generated

ACKs for top commit:
  maflcko:
     lgtm ACK bdb2e8d4ae
  achow101:
    ACK bdb2e8d4ae

Tree-SHA512: 5a19b9892917126980bd3260f6035a8b2c5c9a9cfd16261d5364713ffa4816f1604e8bd3298fcf7ca7be072f33cd5a9f4e0a89cfee77ae90dc0b201e4abc0f3f
2023-10-26 14:37:52 -04:00
Andrew Chow
7be62df80f
Merge bitcoin/bitcoin#26078: p2p: return CSubNet in LookupSubNet
fb3e812277 p2p: return `CSubNet` in `LookupSubNet` (brunoerg)

Pull request description:

  Analyzing the usage of `LookupSubNet`, noticed that most cases uses check if the subnet is valid by calling `subnet.IsValid()`, and the boolean returned by `LookupSubNet` hasn't been used so much, see:
  29d540b7ad/src/httpserver.cpp (L172-L174)
  29d540b7ad/src/net_permissions.cpp (L114-L116)

  It makes sense to return `CSubNet` instead of `bool`.

ACKs for top commit:
  achow101:
    ACK fb3e812277
  vasild:
    ACK fb3e812277
  theStack:
    Code-review ACK fb3e812277
  stickies-v:
    Concept ACK, but Approach ~0 (for now). Reviewed the code (fb3e812277) and it all looks good to me.

Tree-SHA512: ba50d6bd5d58dfdbe1ce1faebd80dd8cf8c92ac53ef33519860b83399afffab482d5658cb6921b849d7a3df6d5cea911412850e08f3f4e27f7af510fbde4b254
2023-10-26 14:29:47 -04:00
Andrew Chow
5572f98f05
Merge bitcoin/bitcoin#28107: util: Type-safe transaction identifiers
940a49978c Use type-safe txid types in orphanage (dergoegge)
ed70e65016 Introduce types for txids & wtxids (dergoegge)
cdb14d79e8 [net processing] Use HasWitness over comparing (w)txids (dergoegge)

Pull request description:

  We currently have two different identifiers for transactions: `txid` (refering to the hash of a transaction without witness data) and `wtxid` (referring to the hash of a transaction including witness data). Both are typed as `uint256` which could lead to type-safety bugs in which one transaction identifier type is passed where the other would be expected.

  This PR introduces explicit `Txid` and `Wtxid` types that (if used) would cause compilation errors for such type confusion bugs.

  (Only the orphanage is converted to use these types in this PR)

ACKs for top commit:
  achow101:
    ACK 940a49978c
  stickies-v:
    ACK 940a49978c
  hebasto:
    ACK 940a49978c, I have reviewed the code and it looks OK.
  instagibbs:
    re-ACK 940a49978c
  BrandonOdiwuor:
    re-ACK 940a49978c
  glozow:
    reACK 940a49978c

Tree-SHA512: 55298d1c2bb82b7a6995e96e554571c22eaf4a89fb2a4d7a236d70e0f625e8cca62ff2490e1c179c47bd93153fe6527b56870198f026f5ee7753d64d7a424c92
2023-10-26 14:18:55 -04:00
dergoegge
cf70a8d565 [net] Check i2p private key constraints
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
2023-10-26 16:50:50 +01:00
Andrew Chow
cb8844e2b9
Merge bitcoin/bitcoin#28728: wallet: [bugfix] Mark CNoDestination and PubKeyDestination constructor explicit
1111475b41 bugfix: Mark CNoDestination and PubKeyDestination constructor explicit (MarcoFalke)
fa5ccc4137 iwyu: Export prevector.h from script.h (MarcoFalke)

Pull request description:

  It seems confusing to allow any script, even one with a corresponding address, to silently convert to `CNoDestination`.

  Make the converstion `explicit` in the code, and fix any bugs that were previously introduced.

  In a follow-up, the class can be renamed, or the documentation can be updated to better reflect what the code does.

ACKs for top commit:
  josibake:
    ACK 1111475b41
  achow101:
    ACK 1111475b41
  furszy:
    Code review ACK 1111475

Tree-SHA512: d8b5f54d0cd8649a31e227ef164bb13e5b81ee9820f1976fd70c7a0de6841fba72d549c2f63e351c8cdda37dceb4763eca203e1c8ef385f46d9da6f1855c39ec
2023-10-26 11:14:40 -04:00
MarcoFalke
faa769db5a
Fix bugprone-lambda-function-name errors
Can be reviewed with

--color-moved=dimmed-zebra
2023-10-26 16:58:36 +02:00
MarcoFalke
1111475b41
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.
2023-10-25 22:46:55 +02:00
Andrew Chow
2a349f9ea5
Merge bitcoin/bitcoin#28264: test: refactor: support sending funds with outpoint result
50d1ac1207 test: remove unused `find_output` helper (Sebastian Falbesoner)
73a339abc3 test: refactor: support sending funds with outpoint result (Sebastian Falbesoner)

Pull request description:

  In wallet-related functional tests we often want to send funds to an address and  use the resulting (non-change) UTXO directly after as input for another transaction. Doing that is currently tedious, as it involves finding the index part of the outpoint manually by calling helpers like `find_vout_for_address` or `find_output` first.  This results in two different txid/vout variables which then again have to be combined to a single dictionary `{"txid": ..., "vout": ...}` in order to be specified as input for RPCs like `createrawtransaction` or `createpsbt`. For example:

  ```
  txid1 = node1.sendtoaddress(addr1, value1)
  vout1 = find_vout_for_address(node1, txid1, addr1)
  txid2 = node2.sendtoaddress(addr2, value2)
  vout2 = find_vout_for_address(node2, txid2, addr2)
  node.createrawtransaction([{'txid': txid1, 'vout': vout1}, {'txid': txid2, 'vout': vout2}], .....)
  ```

  This PR introduces a helper `create_outpoints` to immediately return the outpoint as
  UTXO dictionary in the common format, making the tests more readable and avoiding unnecessary duplication:

  ```
  utxo1 = self.create_outpoints(node1, outputs=[{addr1: value1}])[0]
  utxo2 = self.create_outpoints(node2, outputs=[{addr2: value2}])[0]
  node.createrawtransaction([utxo1, utxo2], .....)
  ```

  Tests are switched to work with UTXO-objects rather than two individual txid/vout variables accordingly.

  The `find_output` helper is removed, as it seems generally a bad idea to search for an outpoint only based on the output value. If that's really ever needed in the future, it makes probably more sense to add it as an additional parameter to `find_vout_of_address`. Note that `find_output` supported specifying a block-hash for where to look for the transaction (being passed on to the `getrawtransaction` RPC). This seems to be unneeded, as txids are always unique and for the only test that used that parameter (rpc_psbt.py) there was no observed difference in run-time, so it was not reintroduced in the new helper.

  There are still some `find_vout_of_address` calls remaining, used for detecting change outputs or for whenever the sending happens via `sendrawtransaction` instead, so this PR tackles not all, but the most common case.

ACKs for top commit:
  achow101:
    ACK 50d1ac1207
  BrandonOdiwuor:
    ACK 50d1ac1207
  maflcko:
    ACK 50d1ac1207 🖨

Tree-SHA512: af2bbf13a56cc840fefc1781390cf51625f1e41b3c030f07fc9abb1181b2d414ddbf795e887db029e119cbe45de14f7c987c0cba72ff0b8953080ee218a7915a
2023-10-25 11:05:38 -04:00
pablomartin4btc
811067ca1c test: add coverage for snapshot chainstate not matching AssumeUTXO parameters
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
2023-10-25 11:14:43 -03:00
ns-xvrn
fe3ac3700d test: replace random_bytes with randbytes #28720 2023-10-25 08:56:41 -04:00
Hennadii Stepanov
64879f4c03
Merge bitcoin-core/gui#771: Avoid error-prone leading whitespace in translatable strings
856325fac1 lint: Add `lint-qt-translation.py` (Hennadii Stepanov)
294a018bf5 qt: Avoid error prone leading spaces in translatable strings (Hennadii Stepanov)
d8298e7f06 qt, refactor: Drop superfluous type conversions (Hennadii Stepanov)

Pull request description:

  While working on the GUI translation via Transifex web interface, I found it error-prone to have leading whitespace in translatable strings. This is because it is very easy to unintentionally drop them in translations unnoticed.

  Fixed all current cases. Added a linter to prevent similar cases in the future.

ACKs for top commit:
  furszy:
    utACK 856325f

Tree-SHA512: b1ca5effb2db6649e1e99382de79acf3a9f81cc9dad434db5623338489e597897e8addd60c1ab3dcc7506ae62753a7a4ad5a41d7a865f8fcdf94348b54baa7e7
2023-10-25 13:20:07 +01:00
Hennadii Stepanov
afa081a39b
Merge bitcoin-core/gui#742: Exit and show error if unrecognized command line args are present
51e4dc49f5 gui: Show error if unrecognized command line args are present (John Moffett)

Pull request description:

  Fixes https://github.com/bitcoin-core/gui/issues/741

  Starting bitcoin-qt with non-hyphen ("-") arguments causes it to silently ignore any later valid options. For instance, invoking `bitcoin-qt -server=1 foo -regtest` on a fresh install will run `mainnet` instead of `regtest`.

  This change makes the client exit with an error message if any such "loose" arguments are encountered. This mirrors how `bitcoind` handles it:

  c6287faae4/src/bitcoind.cpp (L127-L132)

  However, BIP-21 `bitcoin:` payment URIs are still allowed, but only if they're not followed by any additional options.

ACKs for top commit:
  maflcko:
    lgtm ACK 51e4dc49f5
  hernanmarino:
    tested ACK 51e4dc49f5
  pablomartin4btc:
    tACK 51e4dc49f5
  hebasto:
    ACK 51e4dc49f5, I have reviewed the code and it looks OK.

Tree-SHA512: 3997a7a9a747314f13e118aee63e8679e00ed832d9c6f115559a4c39c9c4091572207c60e362cb4c19fc8da980d4b0b040050aa70c5ef84a855cb7e3568bbf13
2023-10-25 13:12:59 +01:00
MarcoFalke
fa5ccc4137
iwyu: Export prevector.h from script.h
This should cut some include bloat and seems fine to do, because
prevector exists primarily to represent scripts.

Also, add missing includes to script.h and addresstype.h
2023-10-25 11:55:50 +02:00
pablomartin4btc
4a5be10b92 assumeutxo, blockstorage: prevent core dump on invalid hash 2023-10-24 23:39:10 -03:00
fanquake
d53400e75e
Merge bitcoin/bitcoin#28627: depends: zeromq 4.3.5
986d7fed05 depends: zeromq 4.3.5 (fanquake)

Pull request description:

  First new point release of zeromq in two and a half years. Mostly bug fixes; the project also completed a relicense to the "Mozilla Public License".

  See https://github.com/zeromq/libzmq/releases/tag/v4.3.5.

ACKs for top commit:
  hebasto:
    ACK 986d7fed05, I have reviewed the code and it looks OK.
  TheCharlatan:
    ACK 986d7fed05

Tree-SHA512: cdd6abfbbe10873c1ca267fed648c2e6ff17a4aff50c414924006e63fa39d501e803f8893a5cd966a2078b5c077f2578e482483e6723ea6f5760f16211d40998
2023-10-24 22:05:06 +01:00
fanquake
ba0313d84b
Merge bitcoin/bitcoin#28210: build: Bump clang minimum supported version to 13
fae379b6b1 build: Bump minimum supported Clang to clang-13 (MarcoFalke)
fab1ef9512 Bump .python-version from 3.9.17 to 3.9.18 (MarcoFalke)

Pull request description:

  All supported operating systems ship with clang-13 (or later), so bump the minimum to that and remove now unused workarounds for previous clang bugs.

  For reference:
  * https://packages.debian.org/bullseye/clang-13
  * https://packages.ubuntu.com/jammy/clang (`clang-14`) and https://packages.ubuntu.com/jammy/clang-15
  * CentOS-like 8/9 Stream: All Clang versions from 13 to 15
  * FreeBSD 12/13: All Clang version from 13 to 16
  * OpenSuse Tumbleweed ships with https://software.opensuse.org/package/clang (`clang16`); No idea about OpenSuse Leap

  This is for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month.

ACKs for top commit:
  fanquake:
    ACK fae379b6b1

Tree-SHA512: 8ed2b227de39b60d3f004daa4a38ea66fe005988bd977046a40613fba847d88d272925732f24777c00264abb99e25874b05b4b9243868d304eba84b450835ccc
2023-10-24 22:03:54 +01:00
MarcoFalke
fae379b6b1
build: Bump minimum supported Clang to clang-13 2023-10-24 18:52:00 +02:00
MarcoFalke
fab1ef9512
Bump .python-version from 3.9.17 to 3.9.18
Requested in https://github.com/bitcoin/bitcoin/pull/28211/files#r1309945635
2023-10-24 18:51:24 +02:00
fanquake
43704827b4
Merge bitcoin/bitcoin#28211: Bump python minimum supported version to 3.9
fa25e8b0a1 doc: Recommend lint image build on every call (MarcoFalke)
faf70c1f33 Bump python minimum version to 3.9 (MarcoFalke)
fa8996b930 ci: Bump i686_multiprocess.sh to latest Ubuntu LTS (MarcoFalke)

Pull request description:

  All supported operating systems ship with python 3.9 (or later), so bumping the minimum should not cause any issues. A bump will allow new code to use new python 3.9 features.

  For reference:
  * https://packages.debian.org/bullseye/python3
  * https://packages.ubuntu.com/focal/python3.9
  * FreeBSD 12/13 also ships with 3.9
  * CentOS-like 8/9 also ships with 3.9 (and 3.11)
  * OpenSuse Leap also ships with 3.9 (and 3.11) https://software.opensuse.org/package/python311-base

  This is for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month.

ACKs for top commit:
  Sjors:
    ACK fa25e8b0a1
  jamesob:
    ACK fa25e8b0a1 ([`jamesob/ackr/28211.1.MarcoFalke.bump_python_minimum_supp`](https://github.com/jamesob/bitcoin/tree/ackr/28211.1.MarcoFalke.bump_python_minimum_supp))

Tree-SHA512: 86c9f6ac4b5ba94a62ee6a6062dd48a8295d8611a39cdb5829f4f0dbc77aaa1a51edccc7a99275bf699143ad3a6fe826de426d413e5a465e3b0e82b86d10c32e
2023-10-24 17:24:30 +01:00
fanquake
986d7fed05
depends: zeromq 4.3.5
https://github.com/zeromq/libzmq/releases/tag/v4.3.5
2023-10-24 12:11:11 +01:00
fanquake
004367dba8
Merge bitcoin/bitcoin#28714: doc: remove release note fragments for 26.x branch
1ac5584f80 doc: remove release note fragments for 26.x branch (fanquake)
799ce4d050 build: bump version to 26.99 (fanquake)

Pull request description:

  26.x has been branched. Delete all release note fragments from this branch.
  Bump master version to 26.99.

ACKs for top commit:
  stickies-v:
    ACK 1ac5584f80

Tree-SHA512: d7d0c26333ed026460fb648ff5cb8f2f3abe150b47dcf011a563b8fcaad6efe59688f3aa2f23c246e003e37d9b612bb9b9f017ad17cf575455e3f73c6948cff8
2023-10-24 11:40:16 +01:00
fanquake
1ac5584f80
doc: remove release note fragments for 26.x branch 2023-10-24 11:13:44 +01:00
fanquake
799ce4d050
build: bump version to 26.99 2023-10-24 11:13:44 +01:00
fanquake
96ec3b67a7
Merge bitcoin/bitcoin#28707: doc: add historical release notes for 24.2
3f482ac231 doc: add historical release notes for 24.2 (fanquake)

Pull request description:

  v24.2 has [been tagged](https://github.com/bitcoin/bitcoin/releases/tag/v24.2).

ACKs for top commit:
  stickies-v:
    ACK 3f482ac231

Tree-SHA512: 71773832910ecda7ed34c6545d184ecbc743d9a36aadd8e4bd367ff60ef5b8048d39335b2347878c4a1a076cc691f12f0e36e8db542757c883d7f83d1161625d
2023-10-24 10:33:26 +01:00
fanquake
ab61087a7e
Merge bitcoin/bitcoin#28660: test: enable reindex readonly test on *BSD
5a0688a20d test: enable reindex readonly test on *BSD and macOS as root (Matthew Zipkin)

Pull request description:

  see https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1349505585

  OpenBSD and FreeBSD don't have `chattr` but they do have `chflags`, use that method to make the block file immutable for the reindex_readonly test.

  Written and tested on a VPS running FreeBSD:
  ```
  FreeBSD freebsd-13-1 13.2-RELEASE-p4 FreeBSD 13.2-RELEASE-p4 GENERIC amd64
  ```

ACKs for top commit:
  maflcko:
    re-cr-lgtm-ACK 5a0688a20d
  jonatack:
    ACK 5a0688a20d tested on macOS only
  theStack:
    ACK 5a0688a20d

Tree-SHA512: 8c88d282d09c00355d22c4c504b779f60e420327a5e07bcf80fa77b97fefcb04952af9ceaf439d9033a0a2448cb26a02663fe6bddcd4a74792857cfbaf1c5162
2023-10-24 10:32:48 +01:00
Sebastian Falbesoner
50d1ac1207 test: remove unused find_output helper 2023-10-24 11:13:57 +02:00
Sebastian Falbesoner
73a339abc3 test: refactor: support sending funds with outpoint result
This commit introduces a helper `create_outpoints` to execute the
`send` RPC and immediately return the target address outpoints as UTXO
dictionary in the common format, making the tests more readable and
avoiding unnecessary duplication.
2023-10-24 11:13:51 +02:00
TheCharlatan
c1144f0076
tests: Reset node context members on ~BasicTestingSetup
The destruction/resetting of node context members in the tests should
roughly follow the behaviour of the Shutdown function in `init.cpp`.
2023-10-24 08:39:45 +02:00
TheCharlatan
9759af17ff
shutdown: Destroy kernel last
Currently the shutdown function resets the kernel before the
chainman and scheduler. Invert this order by resetting the kernel
last, since they might rely on the kernel.
2023-10-24 08:37:22 +02:00
Ryan Ofsky
f06016d77d wallet: Add asserts to detect unset transaction height values
Also document GetTxDepthInMainChain preconditions better
2023-10-23 17:35:36 -04:00