Commit Graph

489 Commits

Author SHA1 Message Date
merge-script 54e406a305
Merge bitcoin/bitcoin#32459: qt: drop unused watch-only functionality
e8661aac75 wallet: drop watch-only things from interface (Sjors Provoost)
e99188e7da qt: drop unused watch-only functionality (Sjors Provoost)

Pull request description:

  The watch-only functionality in the GUI was only used for legacy wallets. Watch-only descriptor wallets do not use this.

  The only visible changes of this PR should be:
  - dropped "Spendable:" label from the overview tab
  - column width cache is reset

  This PR also removes some unused variables from the interface.

ACKs for top commit:
  davidgumberg:
    Review ACK e8661aac75.
  hebasto:
    ACK e8661aac75, I have reviewed the code and it looks OK. The `src/qt/forms/overviewpage.ui` form was reviewed in Qt Designer.

Tree-SHA512: d7edb0f167e0b934075398a76eddca69890bb36848a918c932b1c2cea85ee87285e876cbfdf1f6dec7adf26b9f405fb558c70bec0c84585c0a9df33c2af78393
2025-05-21 10:13:00 +01:00
merge-script ec81204694
Merge bitcoin/bitcoin#31622: psbt: add non-default sighash types to PSBTs and unify sighash type match checking
ee045b61ef rpc, psbt: Require sighashes match for descriptorprocesspsbt (Ava Chow)
2b7682c372 psbt: use sighash type field to determine whether to remove non-witness utxos (Ava Chow)
28781b5f06 psbt: Add sighash types to PSBT when not DEFAULT or ALL (Ava Chow)
15ce1bd73f psbt: Enforce sighash type of signatures matches psbt (Ava Chow)
1f71cd337a wallet: Remove sighash type enforcement from FillPSBT (Ava Chow)
4c7d767e49 psbt: Check sighash types in SignPSBTInput and take sighash as optional (Ava Chow)
a118256948 script: Add IsPayToTaproot() (Ava Chow)
d6001dcd4a wallet: change FillPSBT to take sighash as optional (Ava Chow)
e58b680923 psbt: Return PSBTError from SignPSBTInput (Ava Chow)
2adfd81532 tests: Test PSBT sighash type mismatch (Ava Chow)
5a5d26d612 psbt: Require ECDSA signatures to be validly encoded (Ava Chow)

Pull request description:

  Currently, we do not add the sighash field to PSBTs at all, even when we have signed with a non-default sighash. This PR changes the behavior such that when we (attempt to) sign with a sighash other than DEFAULT or ALL, the sighash type field will be added to the PSBT to inform the later signers that a different sighash type was used by a signer. Notably, this is necessary for MuSig2 support as all signers must sign using the same sighash type, but the sighash is not provided in partial signatures.

  Furthermore, because the sighash type can also be provided on the command line, we require that if both a command line sighash type and the sighash field is present, they must specify the same sighash type. However, this was being checked by the wallet, rather than the signing code, so the `descriptorprocesspsbt` RPC was not enforcing this restriction at all, and in fact ignored the sighash field entirely. This PR refactors the checking code so that the underlying PSBT signing function `SignPSBTInput` does the check.

ACKs for top commit:
  theStack:
    re-ACK ee045b61ef
  rkrux:
    re-ACK ee045b61ef
  fjahr:
    Code review ACK ee045b61ef

Tree-SHA512: 4ead5be1ef6756251b827f594beba868a145d75bf7f4ef6f15ad21f0ae4b8d71b38c83494e5a6b75f37fadd097178cddd93d614b962a2c72fc134f00ba2f74ae
2025-05-21 10:02:49 +01:00
fanquake 7193245cd6
doc: remove For ... comments
We don't add or maintain these, and they are of little value, as
well as having the effect of polluting diffs.

They are also wrong, i.e DEFAULT_SCRIPTCHECK_THREADS is not in
validation.h.
2025-05-19 16:40:33 +01:00
Sjors Provoost e8661aac75
wallet: drop watch-only things from interface 2025-05-19 10:11:18 +02:00
merge-script 51be79c42b
Merge bitcoin/bitcoin#32238: qt, wallet: Convert uint256 to Txid
0671d66a8e wallet, refactor: Convert uint256 to Txid in wallet (marcofleon)
c8ed51e62b wallet, refactor: Convert uint256 to Txid in wallet interfaces (marcofleon)
b3214cefe6 qt, refactor: Convert uint256 to Txid in the GUI (marcofleon)

Pull request description:

  This is part of https://github.com/bitcoin/bitcoin/pull/32189.

  Converts all instances of transactions from `uint256` to `Txid` in the wallet, GUI, and related interfaces.

ACKs for top commit:
  stickies-v:
    re-ACK 0671d66a8e, no changes since 65fcfbb2b38bef20a58daa6c828c51890180611d except rebase.
  achow101:
    ACK 0671d66a8e
  furszy:
    Code review ACK 0671d66a8e

Tree-SHA512: 9fd4675db63195c4eed2d14c25015a1821fb597f51404674e4879a44a9cf18f475021a97c5f62f3926b7783ade5a38567386f663acba9f5861f1f59c1309ed60
2025-05-16 08:54:45 +01:00
Ava Chow d6001dcd4a wallet: change FillPSBT to take sighash as optional
Instead of having the caller have to figure out the correct sane default
to provide to FillPSBT, have FillPSBT do that by having it take the
sighash type as an optional. This further allows it to distinguish
between an explicit sighash type being provided and expecting the
default value to be used.
2025-05-14 14:00:43 -07:00
MarcoFalke ffff949472
remove NotifyWatchonlyChanged
The signal is never called.
2025-05-09 15:03:08 +02:00
MarcoFalke fa62a013a5
remove dead flush()
It is confusing that the chain client flush happens between
StopHTTPServer and StopMapPort. Also, it is unused code. Seems best to
just add it back properly when it is needed again.
2025-05-09 14:59:34 +02:00
marcofleon c8ed51e62b wallet, refactor: Convert uint256 to Txid in wallet interfaces
In most cases throughout the wallet, the implicit conversion from `Txid` to
`const uint256&` works. However, `commitBumpTransaction` requires a `uint256&`
out parameter, so `bumped_txid` in `feebumper::CommitTransaction` is also
updated here to use `Txid`.
2025-05-07 15:59:02 +01:00
marcofleon b3214cefe6 qt, refactor: Convert uint256 to Txid in the GUI
Switch all instances of a transaction from a uint256 to the Txid type.
2025-05-07 15:59:02 +01:00
Ava Chow 83af1a3cca wallet: Delete LegacySPKM
Deletes LegacyScriptPubKeyMan and related tests

Best reviewed with `git diff --patience` or `git diff --histogram`
2025-05-06 16:53:16 -07:00
Ava Chow 99a4ddf5ab
Merge bitcoin/bitcoin#31785: Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods
05117e6e17 rpc: clarify longpoll behavior (Sjors Provoost)
5315278e7c Have createNewBlock() wait for a tip (Sjors Provoost)
64a2795fd4 rpc: handle shutdown during long poll and wait methods (Sjors Provoost)
a3bf43343f rpc: drop unneeded IsRPCRunning() guards (Sjors Provoost)
f9cf8bd0ab Handle negative timeout for waitTipChanged() (Sjors Provoost)

Pull request description:

  This PR prevents Mining interface methods from sometimes crashing when called during startup before a tip is connected. It also makes other improvements like making more RPC methods usable from the GUI. Specifically this PR:

  - Adds an `Assume` check to disallow passing negative timeout values to `Mining::waitTipChanged`
  - Makes `waitfornewblock`, `waitforblock` and `waitforblockheight` RPC methods usable from the GUI when `-server=1` is not set.
  - Changes `Mining::waitTipChanged` to return `optional<BlockRef>` instead of `BlockRef` and return `nullopt` instead of crashing if there is a timeout or if the node is shut down before a tip is connected.
  - Changes `Mining::waitTipChanged` to not time out before a tip is connected, so it is convenient and safe to call during startup, and only returns `nullopt` on early shutdowns.
  - Changes `Mining::createNewBlock` to block and wait for a tip to be connected if it is called on startup instead of crashing. Also documents that it will return null on early shutdowns.

  This allows `waitNext()` (added in https://github.com/bitcoin/bitcoin/pull/31283) to safely assume `TipBlock()` isn't `null`, not even during a scenario of early shutdown.

  Finally this PR clarifies long poll behaviour, mostly by adding code comments, but also through an early `break`.

ACKs for top commit:
  achow101:
    ACK 05117e6e17
  ryanofsky:
    Code review ACK 05117e6e17, just updated a commit message since last review
  TheCharlatan:
    ACK 05117e6e17
  vasild:
    ACK 05117e6e17

Tree-SHA512: 277c285a6e73dfff88fd379298190b264254996f98b93c91c062986ab35c2aa5e1fbfec4cd71d7b29dc2d68e33f252b5cfc501345f54939d6bd78599b71fec04
2025-04-14 14:39:57 -07:00
merge-script dfb7d58108
Merge bitcoin/bitcoin#31897: mining: drop unused -nFees and sigops from CBlockTemplate
226d81f8b7 mining: drop unused -nFees and sigops from CBlockTemplate (Sjors Provoost)
53ad845fb9 test: check fees and sigops in getblocktemplate (Sjors Provoost)

Pull request description:

  For the coinbase `vTxFees` used a dummy value of -nFees.

  Similarly the first `vTxSigOpsCost` entry was calculated from
  the dummy coinbase transaction.

  This was introduced in #2115, but the values were never returned by the RPC or used in a test.

  Drop 'm and add code comments to prevent confusion.

  This PR also adds test coverage for the `fees` and `sigops` fields in `getblocktemplate`, so it closes #32053.

ACKs for top commit:
  ismaelsadeeq:
    re-ACK 226d81f8b7
  ryanofsky:
    Code review ACK 226d81f8b7. New test was added since last review, which seems very cleanly written and fixes some missing coverage.
  glozow:
    ACK 226d81f8b7

Tree-SHA512: 79c534e6bc4810d29114b04dd6db798877732cb473e773bf3cc28f83d14ee3982392587bd0baa39857bd53a79eae3b730d7a7029b08a9b6c3b5c51f86657ca5d
2025-03-25 08:41:59 -04:00
Sjors Provoost 5315278e7c
Have createNewBlock() wait for a tip
- return null on shutdown instead of the last tip
- ignore timeout value node initialization

This allows consumers of BlockTemplate to safely
assume that a tip is connected, instead of having
to account for startup and early shutdown scenarios.
2025-03-24 09:48:49 +01:00
merge-script a799415d84
Merge bitcoin/bitcoin#31904: refactor: modernize outdated trait patterns using helper aliases (C++14/C++17)
4cd95a2921 refactor: modernize remaining outdated trait patterns (Lőrinc)
ab2b67fce2 scripted-diff: modernize outdated trait patterns - values (Lőrinc)
8327889f35 scripted-diff: modernize outdated trait patterns - types (Lőrinc)

Pull request description:

  The use of [`std::underlying_type_t<T>`](https://en.cppreference.com/w/cpp/types/underlying_type) or [`std::is_enum_v<T>`](https://en.cppreference.com/w/cpp/types/is_enum) (and similar ones, introduced in C++14) replace the `typename std::underlying_type<T>::type` and  `std::is_enum<T>::value` constructs (available in C++11).

  The `_t` and `_v` helper alias templates offer a more concise way to extract the type and value directly.

  I've modified the instances I found in the codebase one-by-one (noticed them while investigating https://github.com/bitcoin/bitcoin/pull/31868), and afterwards extracted scripted diff commits to do the trivial ones automatically.
  The last commit contains the values that were easier done manually.

  I've excluded changes from `src/bench/nanobench.h`, `src/leveldb`, `src/minisketch`, `src/span.h` and `src/sync.h` - let me know if you think they should be included instead.

  A few of the code changes can also be reproduced by clang-tidy (but not all of them):
  ```bash
  cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DBUILD_FOR_FUZZING=ON && cmake --build build -j$(nproc)
  run-clang-tidy -quiet -p build -j $(nproc) -checks='-*,modernize-type-traits' -fix $(git grep -lE '::(value|type)' ./src ':(exclude)src/bench/nanobench.h' ':(exclude)src/leveldb' ':(exclude)src/minisketch' ':(exclude)src/span.h' ':(exclude)src/sync.h')
  ```

ACKs for top commit:
  laanwj:
    Concept and code review ACK 4cd95a2921

Tree-SHA512: a4bcf0f267c0f4e02983b4d548ed6f58d464ec379ac5cd1f998b9ec0cf698b53a9f2557a05a342b661f1d94adefc9a0ce2dc8f764d49453aaea95451e2c4c581
2025-03-17 13:10:10 +08:00
Sjors Provoost 64a2795fd4
rpc: handle shutdown during long poll and wait methods
The waitTipChanged() now returns nullopt if the node is shutting down.

Previously it would return the last known tip during shutdown, but
this creates an ambiguous circumstance in the scenario where the
node is started and quickly shutdown, before notifications().TipBlock()
is set.

The getblocktemplate, waitfornewblock and waitforblockheight RPC
are updated to handle this. Existing behavior is preserved.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2025-03-13 12:12:17 +01:00
Sjors Provoost f9cf8bd0ab
Handle negative timeout for waitTipChanged() 2025-03-13 12:12:17 +01:00
Sjors Provoost 226d81f8b7
mining: drop unused -nFees and sigops from CBlockTemplate
For the coinbase vTxFees used a dummy value of -nFees. This
value was never returned by the RPC or used in a test.

Similarly the fist vTxSigOpsCost entry was calculated from
the dummy coinbase transaction.

Drop both and add code comments to prevent confusion.
2025-03-13 11:16:57 +01:00
Lőrinc 8327889f35 scripted-diff: modernize outdated trait patterns - types
The use of e.g. `std::underlying_type_t<T>` replaces the older `typename std::underlying_type<T>::type`.
The `_t` helper alias template (such as `std::underlying_type_t<T>`) introduced in C++14 offers a cleaner and more concise way to extract the type directly.
See https://en.cppreference.com/w/cpp/types/underlying_type for details.

-BEGIN VERIFY SCRIPT-
sed -i -E 's/(typename )?(std::[a-z_]+)(<[^<>]+>)::type\b/\2_t\3/g' $(git grep -l '::type' ./src ':(exclude)src/bench/nanobench.h' ':(exclude)src/leveldb' ':(exclude)src/minisketch' ':(exclude)src/span.h' ':(exclude)src/sync.h')
-END VERIFY SCRIPT-
2025-02-21 10:41:27 +01:00
Sjors Provoost cadbd4137d
miner: have waitNext return after 20 min on testnet
On testnet we need to create a min diff template after 20 min.
2025-02-19 17:20:58 +01:00
Sjors Provoost d4020f502a
Add waitNext() to BlockTemplate interface 2025-02-19 17:20:57 +01:00
merge-script 9491676438
Merge bitcoin/bitcoin#31157: Cleanups to port mapping module post UPnP drop
70398ae05b mapport: make ProcessPCP void (Antoine Poinsot)
9e6cba2988 mapport: remove unnecessary 'g_mapport_enabled' (Antoine Poinsot)
8fb45fcda0 mapport: remove unnecessary 'g_mapport_current' variable (Antoine Poinsot)
1b223cb19b mapport: merge DispatchMapPort into StartMapPort (Antoine Poinsot)
9bd936fa34 mapport: drop unnecessary function (Antoine Poinsot)
2a6536ceda mapport: rename 'use_pcp' to 'enable' (Antoine Poinsot)
c4e82b854c mapport: make 'enabled' and 'current' bool (Antoine Poinsot)

Pull request description:

  Followup to #31130, this does a couple cleanups to `src/mapport.*` to clarify the logic now that there is a single protocol option for port mapping.

ACKs for top commit:
  laanwj:
    Code review ACK 70398ae05b
  TheCharlatan:
    ACK 70398ae05b

Tree-SHA512: d9a3ab4fcd59a7cf4872415c40cc7ac3a98dfc5aa25e195d4df880bb588bac286c30c3471e9d9499de379a75f45dcd0a82019eba3cb9f342004ae1482d0ba075
2025-02-14 11:15:53 +01:00
Fabian Jahr 42d5d53363
interfaces: Add helper function for wallet on pruning 2025-01-05 17:28:19 +01:00
Sjors Provoost c991cea1a0
Remove processNewBlock() from mining interface
processNewBlock was added in 7b4d3249ce, but became unnecessary with the introduction of interfaces::BlockTemplate::submitSolution in 7b4d3249ce.

getTransactionsUpdated() is only needed by the implementation of waitFeesChanged() (not yet part of the interface).
2024-12-18 09:20:26 +07:00
Sjors Provoost 9a47852d88
Remove getTransactionsUpdated() from mining interface
It's unnecessary to expose it via this interface.
2024-12-18 09:19:12 +07:00
Sjors Provoost bfc4e029d4
Remove testBlockValidity() from mining interface
It's very low level and not used by the proposed Template Provider.

This method was introduced in d8a3496b5a
and a74b0f93ef.
2024-12-18 09:18:21 +07:00
Ryan Ofsky a95a8ba3a3
Merge bitcoin/bitcoin#31197: refactor: mining interface 30955 followups
f86678156a Check leaves size maximum in MerkleComputation (Sjors Provoost)
4d57288246 refactor: use CTransactionRef in submitSolution (Sjors Provoost)
2e81791d90 Drop TransactionMerklePath default position arg (Sjors Provoost)
39d3b538e6 Rename merkle branch to path (Sjors Provoost)

Pull request description:

  This PR implements the refactors suggested in https://github.com/bitcoin/bitcoin/pull/30955#pullrequestreview-2354931253.

ACKs for top commit:
  tdb3:
    code review re-ACK f86678156a
  itornaza:
    re ACK f86678156a
  ryanofsky:
    Code review ACK f86678156a only changes since last review are a whitespace change and adding an Assume statement to check for size_t -> uint32_t overflows

Tree-SHA512: 661b5d5d0e24b2269bf33ab1484e37c36e67b32a7796d77ca3b1856d3043378b081ad43c32a8638b46fa8c0de51c823fd9747dd9fc81f958f20d327bf330a47c
2024-12-17 12:32:45 -05:00
Ryan Ofsky cd3d9fa5ea
Merge bitcoin/bitcoin#31318: Drop script_pub_key arg from createNewBlock
52fd1511a7 test: drop scriptPubKeyIn arg from CreateNewBlock (Sjors Provoost)
ff41b9e296 Drop script_pub_key arg from createNewBlock (Sjors Provoost)
7ab733ede4 rpc: rename coinbase_script to coinbase_output_script (Sjors Provoost)

Pull request description:

  Providing a script for the coinbase transaction is only done in test code and for (unoptimized) CPU solo mining.

  Production miners use the `getblocktemplate` RPC which omits the coinbase transaction entirely from its block template, leaving it to external (pool) software to construct it.

  This commit removes the `script_pub_key argument` from `createNewBlock()` in the Mining interface.

  A coinbase script can still be passed via `BlockCreateOptions` instead. Tests are modified to do so.

ACKs for top commit:
  ryanofsky:
    Code review ACK 52fd1511a7. No change since last review other than rebase
  TheCharlatan:
    Re-ACK 52fd1511a7
  vasild:
    ACK 52fd1511a7

Tree-SHA512: c4b3a53774d9a5dc90950e77f47a64dbb68f971baffbb9a0d8f59332ef8e52d0c039130c925bde73135b3d0e79e65d91d1df30dc4cff13f32d8a72e5c56669d8
2024-12-17 11:50:44 -05:00
Sjors Provoost 4d57288246
refactor: use CTransactionRef in submitSolution 2024-12-17 10:12:31 +07:00
Sjors Provoost 37946c0aaf
Set notifications m_tip_block in LoadChainTip()
Ensure KernelNotifications m_tip_block is set even if no new block arrives.

Additionally, have node init always wait for this to happen.
2024-12-06 14:24:21 +07:00
Sjors Provoost ff41b9e296
Drop script_pub_key arg from createNewBlock
Providing a script for the coinbase transaction is only done in test code
and for CPU solo mining.

Production miners use the getblocktemplate RPC which omits the coinbase
transaction entirely from its block template, leaving it to external (pool)
software to construct it.

A coinbase script can still be passed via BlockCreateOptions instead.

A temporary overload is added so that the test can be modified in the
next commit.
2024-12-04 12:44:57 +07:00
Antoine Poinsot 2a6536ceda
mapport: rename 'use_pcp' to 'enable'
There is only a single protocol now, caller should just be concerned about whether to enable port mapping or not.
2024-10-29 11:58:51 -04:00
Antoine Poinsot a5fcfb7385
interfaces: remove now unused 'use_upnp' arg from 'mapPort' 2024-10-24 18:23:30 +02:00
Ava Chow 0c2c3bb3f5
Merge bitcoin/bitcoin#30955: Mining interface: getCoinbaseMerklePath() and submitSolution()
525e9dcba0 Add submitSolution to BlockTemplate interface (Sjors Provoost)
47b4875ef0 Add getCoinbaseMerklePath() to Mining interface (Sjors Provoost)
63d6ad7c89 Move BlockMerkleBranch back to merkle.{h,cpp} (Sjors Provoost)

Pull request description:

  The new `BlockTemplate` interface introduced in #30440 allows for a more efficient way for a miner to submit the block solution. Instead of having the send the full block, it only needs to provide the nonce, timestamp, version fields and coinbase transaction.

  This PR introduces `submitSolution()` for that. It's currently unused.

  #29432 and https://github.com/Sjors/bitcoin/pull/48 use it to process the Stratum v2 message [SubmitSolution](https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#77-submitsolution-client---server). The method should be sufficiently generic to work with alternative mining protocols (none exist that I'm aware off).

  This PR also introduces `getCoinbaseMerklePath()`, which is needed in Stratum v2 to construct the `merkle_path` field of the `NewTemplate` message (see [spec](https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#72-newtemplate-server---client)). The coinbase merkle path is also used in Stratum "v1", see e.g. https://bitcoin.stackexchange.com/questions/109820/questions-on-merkle-root-hashing-for-stratum-pools

  This last function uses `BlockMerkleBranch` which was moved to the test code in #13191. The reason back then for moving it was that it was no longer used. This PR moves it back.

  This PR does not change behaviour since both methods are unused.

ACKs for top commit:
  achow101:
    ACK 525e9dcba0
  itornaza:
    Code review ACK 525e9dcba0
  tdb3:
    Code review and light test ACK 525e9dcba0
  ryanofsky:
    Code review ACK 525e9dcba0. Left minor suggestions but none are important, and looks like this could be merged as-is

Tree-SHA512: 2a6a8f5d409ff4926643193cb67702240c7c687615414371e53383d2c13c485807f65e21e8ed98515b5456eca3d9fca13cec04675814a4081467d88b849c5653
2024-10-09 20:20:09 -04:00
MarcoFalke fa7f52af1a
refactor: Use wait_for predicate to check for interrupt
Also use uint256::ZERO where appropriate for self-documenting code.
2024-10-01 09:11:08 +02:00
MarcoFalke fa4c075033
doc: Clarify waitTipChanged docs
It should be obvious that a wait is not needed if the tip does not
match.

Also, remove a comment that the blockTip notification was only meant for
the "UI". (It is used by other stuff for a long time)
2024-10-01 09:08:37 +02:00
Ava Chow c33eb2360e
Merge bitcoin/bitcoin#30043: net: Replace libnatpmp with built-in PCP+NATPMP implementation
5c7cacf649 ci: Remove natpmp build option and libnatpmp dependency (laanwj)
7e7ec984da doc: Remove mention of natpmp build options (laanwj)
061c3e32a2 depends: Drop natpmp and associated option from depends (laanwj)
20a18bf6aa build: Drop libnatpmp from build system (laanwj)
7b04709862 qt: Changes for built-in PCP+NAT-PMP (laanwj)
52f8ef66c6 net: Replace libnatpmp with built-in NATPMP+PCP implementation in mapport (laanwj)
97c97177cd net: Add PCP and NATPMP implementation (laanwj)
d72df63d16 net: Use GetLocalAddresses in Discover (laanwj)
e02030432b net: Add netif utility (laanwj)
754e425438 crypto: Add missing WriteBE16 function (laanwj)

Pull request description:

  Continues #30005. Closes #17012..

  This PR adds PCP (Port Control Protocol) from [RFC6887](https://datatracker.ietf.org/doc/html/rfc6887).  This adds, in addition to the existing IPv4 port mapping (which now uses PCP, with fallback to NAT-PMP), support for IPv6 pinholing-that is, opening a port on the firewall to make it reachable.

  PCP, like NAT-PMP is a simple UDP-based protocol, and the implementation is self-contained, so this gets rid of lthe libnatpnp dependency without adding a new one. It should otherwise be a drop-in replacement. NAT-PMP fallback is implemented so this will not make router support worse.

  For now it is disabled by default, though in the future (not in this PR) we could consider enable it by default to increase the number of connectable nodes without adding significant attack surface.

  To test:
  ```bash
  bitcoind -regtest -natpmp=1 -debug=net
  ```

  (most of the changes in this PR are, ironically, removing the libnatpmp dependency and associated build system and build docs)

  ## TODO

  - [x] Default gateway discovery on Linux / FreeBSD
  - [x] Default gateway discovery on Windows
  - [x] Default gateway discovery on MacOS
  - [x] Either solve FreeBSD compile issue (probably upstream issue) or remove FreeBSD support

  ## Things to consider for follow-up PRs

  - https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1658764974 avoid unreachable nets (not given to -onlynet=)

  - https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1658949236 could announce an addr:port where we do not listen (no -bind)

  - https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1684368824 could announce the wrong port because it uses GetListenPort()

  - https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1679709347 if we requested one port but another was assigned, then which one to use in the renewal?

  - https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1772017020 Use `GetAdapterAddresses` to discover local addresses for Windows

ACKs for top commit:
  Sjors:
    ACK 5c7cacf649
  achow101:
    ACK 5c7cacf649
  vasild:
    ACK 5c7cacf649

Tree-SHA512: e35b69e56d5f5449a3d48a799f9b7b65107c65eeb3e245c2c1e9d42221e469ca5ead90afae423160601cd664dd553a51c859e04f4492f335b064aae3bf23e3bc
2024-09-30 16:27:47 -04:00
laanwj 52f8ef66c6 net: Replace libnatpmp with built-in NATPMP+PCP implementation in mapport 2024-09-30 11:37:55 +02:00
Sjors Provoost 525e9dcba0
Add submitSolution to BlockTemplate interface 2024-09-26 10:04:45 +02:00
Sjors Provoost 47b4875ef0
Add getCoinbaseMerklePath() to Mining interface 2024-09-26 10:04:45 +02:00
Ava Chow dabc74e86c
Merge bitcoin/bitcoin#30409: Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block
7942951e3f Remove unused g_best_block (Ryan Ofsky)
e3a560ca68 rpc: use waitTipChanged for longpoll (Ryan Ofsky)
460687a09c Remove unused CRPCSignals (Sjors Provoost)
dca923150e Replace RPCNotifyBlockChange with waitTipChanged() (Sjors Provoost)
2a40ee1121 rpc: check for negative timeout arg in waitfor* (Sjors Provoost)
de7c855b3a rpc: recommend -rpcclienttimeout=0 for waitfor* (Sjors Provoost)
77ec072925 rpc: fix waitfornewblock description (Sjors Provoost)
285fe9fb51 rpc: add test for waitforblock and waitfornewblock (Sjors Provoost)
b94b27cf05 Add waitTipChanged to Mining interface (Sjors Provoost)
7eccdaf160 node: Track last block that received a blockTip notification (Sjors Provoost)
ebb8215f23 Rename getTipHash() to getTip() and return BlockRef (Sjors Provoost)
89a8f74bbb refactor: rename BlockKey to BlockRef (Sjors Provoost)

Pull request description:

  This continues the work in #30200 so that a future Stratum v2 Template Provider (see #29432) can avoid accessing node internals. It needs to know when a new block arrives in order to push new templates to connected clients.

  `waitTipChanged()` uses a new kernel notification `notifications().m_tip_block_mutex`, which this PR also introduces (a previous version used `g_best_block`).

  In order to ensure the new method works as intended, the `waitfornewblock`, `waitforblock` and `waitforblockheight` RPC methods are refactored to use it. This allows removing `RPCNotifyBlockChange`.

  There's a commit to add (direct) tests for the methods that are about to be refactored:
  - `waitfornewblock` was already implicitly tested by `feature_shutdown.py`.
  - `waitforblockheight` by `feature_coinstatsindex.py` and `example_test.py`

  This PR renames `getTipHash()` to `getTip()` and returns a `BlockRef` (renamed from `BlockKey`) so that callers can use either the height or hash.

  The later commits make trivial improvements to the `waitfor*` RPC calls (not needed for this PR).

  The `waitTipChanged()` method could probably also be used for the longpoll functionality in `getblocktemplate`, but I'm a bit reluctant to touch that.

  `RPCServer::OnStarted` no longer does anything and `RPCServer::OnStopped` merely prints a log statement. They were added in #5711 as a refactor. This PR drops them entirely.

  Finally `g_best_block` is also dropped.

ACKs for top commit:
  achow101:
    ACK 7942951e3f
  ryanofsky:
    Code review ACK 7942951e3f. Just rebased since last review
  TheCharlatan:
    Re-ACK 7942951e3f

Tree-SHA512: a5559446b4000c95e07aad33284b7ee2e57aafd87e1ae778b3825d59689566d047a8047e47a10f76e6e341e7dc72fd265a65afbc0a9c011d17c4cafd55031837
2024-09-23 15:40:33 -04:00
Ava Chow a8a2628b7a
Merge bitcoin/bitcoin#30828: interfaces: #30697 follow ups
8466329127 chain: simplify `deleteRwSettings` code and improve it's doc (ismaelsadeeq)
f8d91f49c7 chain: dont check for null settings value in `overwriteRwSetting` (ismaelsadeeq)
df601993f2 chain: ensure `updateRwSetting` doesn't update to a null settings (ismaelsadeeq)
c8e2eeeffb chain: uniformly use `SettingsAction` enum in settings methods (ismaelsadeeq)
1e9e735670 chain: move new settings safely in `overwriteRwSetting` (ismaelsadeeq)
1c409004c8 test: remove wallet context from `write_wallet_settings_concurrently` (ismaelsadeeq)

Pull request description:

  This PR addresses the remaining review comments from #30697

  1. Disallowed overwriting settings values with a `null` value.
  2. Uniformly used the `SettingsAction` enum in all settings methods instead of a boolean parameter.
  3. Updated `overwriteRwSetting` to receive the `common::SettingsValue` parameter by value, enabling it to be moved safely.
  4. Removed wallet context from the `write_wallet_settings_concurrently` unit test, as it is not needed.

ACKs for top commit:
  achow101:
    ACK 8466329127
  ryanofsky:
    Code review ACK 8466329127. Looks good, thanks for taking suggestions and applying them to the right commits. Only changes since last review were documentation improvements and simplifying delete method.
  furszy:
    Code review ACK 8466329127

Tree-SHA512: baf2f59ed5aac4a4bda0c84fb6554a466a40d1f7b52b61dc2ff293d83ae60e82b925b7003237b633fecb65eba3a4c108e69166046895d1295809fbe0de67b052
2024-09-20 13:26:38 -04:00
Sjors Provoost b94b27cf05
Add waitTipChanged to Mining interface
Co-authored-by: TheCharlatan <seb.kung@gmail.com>
2024-09-17 09:27:42 +02:00
Sjors Provoost ebb8215f23
Rename getTipHash() to getTip() and return BlockRef 2024-09-17 09:23:59 +02:00
Sjors Provoost 89a8f74bbb
refactor: rename BlockKey to BlockRef 2024-09-17 09:14:15 +02:00
Sjors Provoost dd87b6dff3
Have createNewBlock return BlockTemplate interface
An external program that uses the Mining interface may need quick access to some information in the block template, while it can wait a bit longer for the full raw transaction data.

This would be the case for a Stratum v2 Template Provider which needs to send a NewTemplate message (which doesn't include transactions) as quickly as possible.
2024-09-13 10:14:53 +02:00
ismaelsadeeq 8466329127
chain: simplify `deleteRwSettings` code and improve it's doc
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-09-11 17:04:29 +01:00
ismaelsadeeq f8d91f49c7
chain: dont check for null settings value in `overwriteRwSetting`
- Just call updateRwSetting it will erase the settings when the new
  value is null.
2024-09-11 17:04:28 +01:00
ismaelsadeeq df601993f2
chain: ensure `updateRwSetting` doesn't update to a null settings
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-09-11 16:58:40 +01:00
ismaelsadeeq c8e2eeeffb
chain: uniformly use `SettingsAction` enum in settings methods 2024-09-08 20:37:45 +01:00