Age | Commit message (Collapse) | Author |
|
If not accepting I2P connections, then do not create
`CConnman::m_i2p_sam_session`.
When opening a new outbound I2P connection either use
`CConnman::m_i2p_sam_session` like before or create a temporary one and
store it in `CNode` for destruction later.
|
|
and destroy it when `CNode::m_sock` is closed.
I2P transient sessions are created per connection (i.e. per `CNode`) and
should be destroyed when the connection is closed. Storing the session
in `CNode` is a convenient way to destroy it together with the connection
socket (`CNode::m_sock`).
An alternative approach would be to store a list of all I2P sessions in
`CConnman` and from `CNode::CloseSocketDisconnect()` to somehow ask the
`CConnman` to destroy the relevant session.
|
|
Instead of providing our destination (private key) to the I2P proxy when
creating the session, ask it to generate one for us and do not save it
on disk.
|
|
validation function
fa8671018766b2f0e18c94cff3ab2a67c6b3a41d Clarify that CheckSequenceLocksAtTip is a validation function (MarcoFalke)
Pull request description:
It has been pointed out that a bug in this function can prevent block template creation. ( https://github.com/bitcoin/bitcoin/pull/24080#issuecomment-1065148776 ) So it seems that the scope of this function is more than "policy". Rename it back to "validation", to partially revert commit fa4e30b0f36f2e7a09db7d30dca9008ed9dbcb35.
ACKs for top commit:
ajtowns:
ACK fa8671018766b2f0e18c94cff3ab2a67c6b3a41d - looks fine to me
glozow:
ACK fa8671018766b2f0e18c94cff3ab2a67c6b3a41d
Tree-SHA512: 2e0df8c70df4cbea857977f140a8616cfa7505e74df66c9c9fbcf184670ce3ce7567183c3f76e6f3fe8ca6de0e065b9babde6352d6cb495e71ea077ddedbc3f4
|
|
ff52b24e5c9b3dca269e66e0d78051a2aa5f1462 qt: Update translation source file (Hennadii Stepanov)
15f762fc65af5bd34f4ea52f2b43bcc86dcea80a qt: Bump Transifex slug for 24.x (Hennadii Stepanov)
Pull request description:
Required to open Transifex translations for 24.0 (see bitcoin/bitcoin#24987).
ACKs for top commit:
laanwj:
Changes-match-release-process ACK ff52b24e5c9b3dca269e66e0d78051a2aa5f1462
jarolrod:
ACK ff52b24e5c9b3dca269e66e0d78051a2aa5f1462
Tree-SHA512: f3e65b1608818084f4a3adddd2a58541ebe91ebcdb3717da2eb6f4147a0fc5f0d536a2e9f8b4eacc2a580b12c619d9eec391bfdcc5e81fa02f527408ec73a984
|
|
`{LoadActive,Deactivate}ScriptPubKeyMan` log
b5a762a35368ad5ab07018e5da14229291a54b94 wallet: improve `{LoadActive,Deactivate}ScriptPubKeyMan` log (w0xlt)
Pull request description:
This PR includes the output type description in the log. It currently shows the enum position, which is only useful if the reader knows the code.
Master:
```
Setting spkMan to active: id = 9f..04, type = 3, internal = 0
Setting spkMan to active: id = 3d..21, type = 2, internal = 0
Setting spkMan to active: id = 69..d4, type = 0, internal = 1
Setting spkMan to active: id = 97..ea, type = 1, internal = 1
```
PR:
```
Setting spkMan to active: id = 6a..4f, type = bech32m, internal = false
Setting spkMan to active: id = 83..dc, type = legacy, internal = true
Setting spkMan to active: id = 7e..5d, type = p2sh-segwit, internal = true
Setting spkMan to active: id = bd..d2, type = bech32, internal = true
Setting spkMan to active: id = 13...7c, type = bech32m, internal = true
```
ACKs for top commit:
S3RK:
Code review ACK b5a762a35368ad5ab07018e5da14229291a54b94
achow101:
ACK b5a762a35368ad5ab07018e5da14229291a54b94
theStack:
Code-review ACK b5a762a35368ad5ab07018e5da14229291a54b94
Tree-SHA512: 5a79706d5452e523b0456fb8435545c6c8e550b6722c0d7966af79011275a97ed97cab297562e031d601aa855118082c5b770af118783b1faaaec0cba9f9ee6a
|
|
|
|
|
|
reducing duplicated operations
bc886fcb31e1afa7bbf7b86bfd93e51da7076ccf Change mapWallet to be a std::unordered_map (Andrew Chow)
272356024db978c92112167f8d8e4cc62adad63d Change getWalletTxs to return a set instead of a vector (Andrew Chow)
97532867cf51db3e941231fbdc60f9f4fa0012a0 Change mapTxSpends to be a std::unordered_multimap (Andrew Chow)
1f798fe85ba952273005f68e36ed48cfc36f4c9d wallet: Cache SigningProviders (Andrew Chow)
8a105ecd1aeff15f84c3883e2762bf71ad59d920 wallet: Use CalculateMaximumSignedInputSize to indicate solvability (Andrew Chow)
Pull request description:
While running my coin selection simulations, I noticed that towards the end of the simulation, the wallet would become slow to make new transactions. The wallet generally performs much more slowly when there are a large number of transactions and/or a large number of keys. The improvements here are focused on wallets with a large number of transactions as that is what the simulations produce.
Most of the slowdown I observed was due to `DescriptorScriptPubKeyMan::GetSigningProvider` re-deriving keys every time it is called. To avoid this, it will now cache the `SigningProvider` produced so that repeatedly fetching the `SigningProvider` for the same script will not result in the same key being derived over and over. This has a side effect of making the function non-const, which makes a lot of other functions non-const as well. This helps with wallets with lots of address reuse (as my coin selection simulations are), but not if addresses are not reused as keys will end up needing to be derived the first time `GetSigningProvider` is called for a script.
The `GetSigningProvider` problem was also exacerbated by unnecessarily fetching a `SigningProvider` for the same script multiple times. A `SigningProvider` is retrieved to be used inside of `IsSolvable`. A few lines later, we use `GetTxSpendSize` which fetches a `SigningProvider` and then calls `CalculateMaximumSignedInputSize`. We can avoid a second call to `GetSigningProvider` by using `CalculateMaximumSignedInputSize` directly with the `SigningProvider` already retrieved for `IsSolvable`.
There is an additional slowdown where `ProduceSignature` with a dummy signer is called twice for each output. The first time is `IsSolvable` checks that `ProduceSignature` succeeds, thereby informing whether we have solving data. The second is `CalculateMaximumSignedInputSize` which returns -1 if `ProduceSignature` fails, and returns the input size otherwise. We can reduce this to one call of `ProduceSignature` by using `CalculateMaximumSignedInputSize`'s result to set `solvable`.
Lastly, a lot of time is spent looking in `mapWallet` and `mapTxSpends` to determine whether an output is already spent. The performance of these lookups is slightly improved by changing those maps to use `std::unordered_map` and `std::unordered_multimap` respectively.
ACKs for top commit:
Xekyo:
ACK bc886fcb31e1afa7bbf7b86bfd93e51da7076ccf
furszy:
diff re-reACK bc886fcb
Tree-SHA512: fd710fe1224ef67d2bb83d6ac9e7428d9f76a67f14085915f9d80e1a492d2c51cb912edfcaad1db11c2edf8d2d97eb7ddd95bfb364587fb1f143490fd72c9ec1
|
|
db10cf8ae36693cb4d3ed1b47b84709cf9c0d849 rpc/wallet: add simulaterawtransaction RPC (Karl-Johan Alm)
701a64f548662e01821765b2934b6e4b321fda6d test: add support for Decimal to assert_approx (Karl-Johan Alm)
Pull request description:
(note: this was originally titled "add analyzerawtransaction RPC")
This command iterates over the inputs and outputs of the given transactions, and tallies up the balance change for the given wallet. This can be useful e.g. when verifying that a coin join like transaction doesn't contain unexpected inputs that the wallet will then sign for unintentionally.
I originally proposed this to Elements (https://github.com/ElementsProject/elements/pull/1016) and it was suggested that I propose this upstream.
There is an alternative #22776 to instead add this info to `getbalances` when providing an optional transaction as argument.
ACKs for top commit:
jonatack:
ACK db10cf8ae36693cb4d3ed1b47b84709cf9c0d849
achow101:
re-ACK db10cf8ae36693cb4d3ed1b47b84709cf9c0d849
Tree-SHA512: adf222ec7dcdc068d007ae6f465dbc35b692dc7bb2db337be25340ad0c2f9c64cfab4124df23400995c700f41c83c29a2c34812121782c26063b100c7969b89d
|
|
acbea66589100fe6ef726f4b2a92ec26132ef17b rest: clean-up for `mempool` endpoints (brunoerg)
Pull request description:
The functions `rest_mempool_info` and `rest_mempool_contents` are similar, the only difference between them is:
`rest_mempool_info` uses `MempoolInfoToJSON` to get the mempool informations and `rest_mempool_contents` uses `MempoolToJSON`, for this reason this PR creates a new function to handle it and reduce duplicated code.
Also,
1. Rename `strURIPart` to `str_uri_part`.
2. Rename `strJSON` to `str_json`.
ACKs for top commit:
stickies-v:
re-ACK acbea66589100fe6ef726f4b2a92ec26132ef17b - verified that just the error message was updated since https://github.com/bitcoin/bitcoin/commit/da0c612c3d69164da19332167c38bfcd1f9777a8
theStack:
re-ACK acbea66589100fe6ef726f4b2a92ec26132ef17b
Tree-SHA512: 35f6f0732a573fe8a6cdcc782f89ae3427a1de19f069a68c9c51bb525118c2b07e20303cbe19b9d4b7d1ad055d69c32def2d0fb8f886c851da562dd9ce33ad6a
|
|
a23cca56c0a7f4a267915b4beba3af3454c51603 refactor: Replace BResult with util::Result (Ryan Ofsky)
Pull request description:
Rename `BResult` class to `util::Result` and update the class interface to be more compatible with `std::optional` and with a full-featured result class implemented in https://github.com/bitcoin/bitcoin/pull/25665. Motivation for this change is to update existing `BResult` usages now so they don't have to change later when more features are added in https://github.com/bitcoin/bitcoin/pull/25665.
This change makes the following improvements originally implemented in https://github.com/bitcoin/bitcoin/pull/25665:
- More explicit API. Drops potentially misleading `BResult` constructor that treats any bilingual string argument as an error. Adds `util::Error` constructor so it is never ambiguous when a result is being assigned an error or non-error value.
- Better type compatibility. Supports `util::Result<bilingual_str>` return values to hold translated messages which are not errors.
- More standard and consistent API. `util::Result` supports most of the same operators and methods as `std::optional`. `BResult` had a less familiar interface with `HasRes`/`GetObj`/`ReleaseObj` methods. The Result/Res/Obj naming was also not internally consistent.
- Better code organization. Puts `src/util/` code in the `util::` namespace so naming reflects code organization and it is obvious where the class is coming from. Drops "B" from name because it is undocumented what it stands for (bilingual?)
- Has unit tests.
ACKs for top commit:
MarcoFalke:
ACK a23cca56c0a7f4a267915b4beba3af3454c51603 🏵
jonatack:
ACK a23cca56c0a7f4a267915b4beba3af3454c51603
Tree-SHA512: 2769791e08cd62f21d850aa13fa7afce4fb6875a9cedc39ad5025150dbc611c2ecfd7b3aba8b980a79fde7fbda13babdfa37340633c69b501b6e89727bad5b31
|
|
|
|
network time
fadd8b2676f6d68ec87189871461c9a6a6aa3cac addrman: Use system time instead of adjusted network time (MarcoFalke)
Pull request description:
This changes addrman to use system time for address relay instead of the network adjusted time.
This is an improvement, because network time has multiple issues:
* It is non-monotonic, even if the system time is monotonic.
* It may be wrong, even if the system time is correct.
* It may be wrong, if the system time is wrong. For example, when the node has limited number of connections (`4`), or the system time is wrong by too much (more than +-70 minutes), or the system time only got wrong after timedata collected more than half of the entries while the time was correct, ...)
This may slightly degrade addr relay for nodes where timedata successfully adjusted the time. Addr relay can already deal with minor offsets of up to 10 minutes. Offsets larger than this should still allow addr relay and not result in a DoS.
ACKs for top commit:
dergoegge:
Code review ACK fadd8b2676f6d68ec87189871461c9a6a6aa3cac
Tree-SHA512: b6c178fa01161544e5bc76c4cb23e11bcc30391f7b7a64accce864923766647bcfce2e8ae21d36fb1ffc1afa07bc46415aca612405bd8d4cc1f319c92a08498f
|
|
This command iterates over the inputs and outputs of the given transactions, and tallies up the balance change for the given wallet. This can be useful e.g. when verifying that a coin join like transaction doesn't contain unexpected inputs that the wallet will then sign for unintentionally.
|
|
b01f336708019f8c8274ea701d3446e4123e7af2 util, refactor: Drop explicit conversion to fs::path (Hennadii Stepanov)
138c668e2b4d64279ddefbe07c1d9b7c3d3c537c util, refactor: Use GetPathArg to read "-rpccookiefile" value (Hennadii Stepanov)
1276090705060fcc97072481c2383bbaaa556194 util, refactor: Use GetPathArg to read "-conf" value (Hennadii Stepanov)
Pull request description:
This PR is a continuation of bitcoin/bitcoin#24265 and bitcoin/bitcoin#24306.
Now the following command-line arguments / configure options been read with the `GetPathArg` method:
- `-conf`, also `includeconf` values been normalized
- `-rpccookiefile`
ACKs for top commit:
jarolrod:
Code Review ACK b01f336708019f8c8274ea701d3446e4123e7af2
ryanofsky:
Code review ACK b01f336708019f8c8274ea701d3446e4123e7af. Changes since last review: just dropping first commit (NormalizedPathFromString) as suggested
Tree-SHA512: 2d26d50b73542acdbcc63a32068977b2a49a017d31ca337471a0446f964eb0a6e3e4e3bb1ebe6771566a260f2cae3bc2ebe93b4b523183cea0d51768daab85c9
|
|
faab8dceb37a944d0763fcca390342111e6a9fcc Remove unused SetTip(nullptr) code (MacroFake)
Pull request description:
Now that this path is no longer used after commit b51e60f91472da5216116626afc032acd5616e85, we can remove it.
Future code should reset `CChain` by simply discarding it and constructing a fresh one.
ACKs for top commit:
ryanofsky:
Code review ACK faab8dceb37a944d0763fcca390342111e6a9fcc. Just moved an assert statement since last review
Tree-SHA512: 7dc273b11133d85d32ca2a69c0c7c07b39cdd338141ef5b51496e7de334a809864d5459eb95535497866c8b1e468aae84ed8f91b543041e6ee20130d5622874e
|
|
|
|
For some reason, the primary consumer of getWalletTxs requires the
transactions to be in hash order when it is processing them. std::map
will iterate in hash order so the transactions end up in that order when
placed into the vector. To ensure this order when mapWallet is no longer
ordered, the vector is replaced with a set which will maintain the hash
order.
|
|
|
|
In order to avoid constantly re-deriving the same keys in
DescriptorScriptPubKeyMan, cache the SigningProviders generated inside
of GetSigningProvider.
|
|
...also move the 0-clamping logic to ApplyArgsManOptions, where it
belongs.
|
|
Also:
- Make DEFAULT_MAX_SIG_CACHE_SIZE into constexpr
DEFAULT_MAX_SIG_CACHE_BYTES to utilize the compile-time integer
arithmetic overflow checking available to constexpr.
- Fix comment (MiB instead of MB) for DEFAULT_MAX_SIG_CACHE_BYTES.
- Pass in max_size_bytes parameter to InitS*Cache(), modify log line to
no longer allude to maxsigcachesize being split evenly between the two
validation caches.
- Fix possible integer truncation and add a comment.
[META] I've kept the integer types as int64_t in order to not introduce
unintended behaviour changes, in the next commit we will make
them size_t.
|
|
This fixes an potential overflow which existed prior to this patchset.
If CuckooCache::cache<Element, Hash>::setup_bytes is called with a
`size_t bytes` which, when divided by sizeof(Element), does not fit into
an uint32_t, the implicit conversion to uint32_t in the call to setup
will result in an overflow.
At least on x86_64, this overflow is possible:
static_assert(std::numeric_limits<size_t>::max() / 32 <= std::numeric_limits<uint32_t>::max());
static_assert(std::numeric_limits<size_t>::max() / 4 <= std::numeric_limits<uint32_t>::max());
This commit detects such cases and signals to callers that the `size_t
bytes` input is too large.
|
|
1. -maxsigcachesize is a DEBUG_ONLY option
2. Almost 7 years has passed since its semantics change in
830e3f3d027ba5c8121eed0f6a9ce99961352572 from "number of entries" to
"number of mebibytes"
3. A std::new_handler was added to the codebase after the original PR
which introduced this limit, which will terminate immediately instead
of causing trouble by being caught somewhere unexpected.
|
|
Returning the approximate total size eliminates the need for
InitS*Cache() to do nElems*sizeof(uint256). The cuckoocache has a better
idea of this information.
|
|
In src/test/fuzz/script_sigcache.cpp, we should really be setting up a
full working BasicTestingSetup. The initialize_ function is only run
once anyway.
In src/test/txvalidationcache_tests.cpp, the Dersig100Setup inherits
from BasicTestingSetup, which should have already set up a global script
execution cache without the need to explicitly call
InitScriptExecutionCache.
|
|
Rename `BResult` class to `util::Result` and update the class interface to be
more compatible with `std::optional` and with a full-featured result class
implemented in https://github.com/bitcoin/bitcoin/pull/25665. Motivation for
this change is to update existing `BResult` usages now so they don't have to
change later when more features are added in #25665.
This change makes the following improvements originally implemented in #25665:
- More explicit API. Drops potentially misleading `BResult` constructor that
treats any bilingual string argument as an error. Adds `util::Error`
constructor so it is never ambiguous when a result is being assigned an error
or non-error value.
- Better type compatibility. Supports `util::Result<bilingual_str>` return
values to hold translated messages which are not errors.
- More standard and consistent API. `util::Result` supports most of the same
operators and methods as `std::optional`. `BResult` had a less familiar
interface with `HasRes`/`GetObj`/`ReleaseObj` methods. The Result/Res/Obj
naming was also not internally consistent.
- Better code organization. Puts `src/util/` code in the `util::` namespace so
naming reflects code organization and it is obvious where the class is coming
from. Drops "B" from name because it is undocumented what it stands for
(bilingual?)
- Has unit tests.
|
|
|
|
ddddd6913b1bdee1cad89a32d363306ea1f7b8d7 sort after scripted-diff (MacroFake)
fac812ca835e0d843aba1d4db0e49d183018a29e scripted-diff: Move mempool_args to src/node (MacroFake)
66664384a6fec39ecb4d8d06db66a4f193a06e33 Remove ::g_max_datacarrier_bytes global (MacroFake)
fad0b4fab849eb5f1f0aa54ebc290f85a473ec91 Pass datacarrier setting into IsStandard (MacroFake)
fa2a6b8516b24d7e9ca11926a49cf2b07f661e81 Combine datacarrier globals into one (MacroFake)
fa477d32eefcc3dd2f06b452066290d9936d8c5d Remove ::GetVirtualTransactionSize() alias (MacroFake)
fa2f6c1a611dffe5a3f63fe1b453f1dd420371b1 Remove ::fIsBareMultisigStd global (MacroFake)
fadc14e4f514e7167723285e0ac3d4a7149bbee6 Remove ::dustRelayFee (MacroFake)
fa8a7f01fe1b6db98097021276ed5d929faadbec Remove ::IsStandardTx(tx, reason) alias (MacroFake)
fa7a9114e59b81b50584311a4ab2b3e9a8d956bd test: Remove unused cs_main (MacroFake)
fa9cba7afb73c01bd2c8fefd662dfc80dd98c5e8 Remove ::incrementalRelayFee and ::minRelayTxFee globals (MacroFake)
fa148602e67fe035b1b21eff6c0b656919ac2d45 Remove ::fRequireStandard global (MacroFake)
fa468bdfb62dec286cb977db78d3e47b64dafeba Return optional error from ApplyArgsManOptions (MacroFake)
Pull request description:
This change is good because:
* It moves module-specific init-logic out of the bloated init.cpp
* It removes a global from validation.cpp and places it into the data structure that needs it (mempool)
ACKs for top commit:
glozow:
re ACK ddddd69
ryanofsky:
Code review ACK ddddd6913b1bdee1cad89a32d363306ea1f7b8d7
ariard:
Light Code Review ACK ddddd69
Tree-SHA512: 9de2ce601cfcaa4dfd7d1c92270568895ce8702ccdffb59829fbe9618eab0fd88d738afef33ed66988c66861115e0340e881056bfb71e2aed4af2440bd37eb1e
|
|
|
|
state during chain sync
9e04cfaa76cf9dda27f10359dd43e78dd3268e09 test: add coverage for wallet inconsistent state during sync (furszy)
77de5c693ffe8dc0afa5e40126e9b0e9cc547e04 wallet: guard and alert about a wallet invalid state during chain sync (furszy)
Pull request description:
Follow-up work to my comment in #25239.
Guarding and alerting the user about a wallet invalid state during chain synchronization.
#### Explanation
if the `AddToWallet` tx write fails, the method returns a wtx `nullptr` without removing the recently added transaction from the wallet's map.
Which makes that `AddToWalletIfInvolvingMe` return false (even when the tx is on the wallet's map already), --> which makes `SyncTransaction` skip the `MarkInputsDirty` call --> which leads to a wallet invalid state where the inputs of this new transaction are not marked dirty, while the transaction that spends them still exist on the in-memory wallet tx map.
Plus, as we only store the arriving transaction inside `AddToWalletIfInvolvingMe` when we synchronize/scan block/s from the chain and nowhere else, it makes sense to treat the transaction db write error as a runtime error to notify the user about the problem. Otherwise, the user will lose all the not stored transactions after a wallet shutdown (without be able to recover them automatically on the next startup because the chain sync would be above the block where the txs arrived).
Note:
On purpose, the first commit adds test coverage for it. Showing how the wallet can end up in an invalid state. The second commit corrects it with the proposed solution.
ACKs for top commit:
achow101:
re-ACK 9e04cfaa76cf9dda27f10359dd43e78dd3268e09
jonatack:
ACK 9e04cfaa76cf9dda27f10359dd43e78dd3268e09
Tree-SHA512: 81f765eca40547d7764833d8ccfae686b67c7728c84271bc00dc51272de643dafc270014079dcc9727b47577ba67b340aeb5f981588b54e69a06abea6958aa96
|
|
|
|
It is part of the node library. Also, it won't be moved to the kernel
lib, as it will be pruned of ArgsManager.
-BEGIN VERIFY SCRIPT-
# Move module
git mv src/mempool_args.cpp src/node/
git mv src/mempool_args.h src/node/
# Replacements
sed -i 's:mempool_args\.h:node/mempool_args.h:g' $(git grep -l mempool_args)
sed -i 's:mempool_args\.cpp:node/mempool_args.cpp:g' $(git grep -l mempool_args)
sed -i 's:MEMPOOL_ARGS_H:NODE_MEMPOOL_ARGS_H:g' $(git grep -l MEMPOOL_ARGS_H)
-END VERIFY SCRIPT-
|
|
|
|
|
|
|
|
Each alias is only used in one place.
|
|
|
|
|
|
Apart from tests, it is only used in one place, so there is no need for
an alias.
|
|
|
|
|
|
|
|
Also pass in a (for now unused) reference to the params.
Both changes are needed for the next commit.
|
|
return value
fa7bef2e80c9c290b3e97114cfa7afdea5cbd53c univalue: Remove unused and confusing set*() return value (MacroFake)
Pull request description:
The value is:
* currently unused, and useless without `[[nodiscard]]`
* confusing, because it is always `true`, unless a num-string is set
Instead of adding `[[nodiscard]]`, throw when setting is not possible.
ACKs for top commit:
shaavan:
ACK fa7bef2e80c9c290b3e97114cfa7afdea5cbd53c
aureleoules:
ACK fa7bef2e80c9c290b3e97114cfa7afdea5cbd53c.
Tree-SHA512: 0d74f96f34cb93b66019ab75e12334c964630cc83434f22e58cc7a4fff2ee96a5767e42ab37f08acb67aeacba6811b09c75f1edc68d5e903ccfc59b1c82de891
|
|
NodeImpl/ChainImpl/ExternalSignerImpl members public, rm temporaries, simplify
4bedfd702ad878645c51bea6ee8ce40d8c0bd3da refactor: remove unneeded temporaries in node/interfaces, simplify code (Jon Atack)
b27ba169ebd4a8e4ec29be590f03a4d0da61a0cc refactor: make all NodeImpl/ChainImpl/ExternalSignerImpl members public (Jon Atack)
Pull request description:
- Make all `NodeImpl`, `ChainImpl` and `ExternalSignerImpl` class members `public` (and document why), to be consistent in all the `*Impl` classes in `src/node/interfaces.cpp` and `src/wallet/interfaces.cpp` and to help future reviewers and contributors.
- Remove unneeded temporaries in `NodeImpl` and `ChainImpl` methods in `src/node/interfaces.cpp` and simplify, to make the code easier to read and understand and to improve performance by avoiding unnecessary move operations.
ACKs for top commit:
ryanofsky:
Code review ACK 4bedfd702ad878645c51bea6ee8ce40d8c0bd3da. Changes since last review, applying suggested style & simplifiying first commit. Also avoiding another lock in second commit.
Tree-SHA512: 112f7cad5e2838c94c5b79d61328f42fe75fdb97f401ab49eccf696fc2c6a8a0c0ee55ec974c0602acf7423f78bb82e90eb8a0cc531e1d3347f73b7c83685504
|
|
`CCoinsViewCache::AddCoin`
f8e228476f54b36beacc3fd09038dfeebdac6b3c tracing: do not use `coin` after move in `CCoinsViewCache::AddCoin` (Seibart Nedor)
Pull request description:
This is fix for https://github.com/bitcoin/bitcoin/issues/25640.
ACKs for top commit:
0xB10C:
ACK f8e228476f54b36beacc3fd09038dfeebdac6b3c
Tree-SHA512: e7643ac8e6b6247aaf250f44572c4b458da4aea030ac0268227564e6857200e9c23efe325cfc535f46498cbeccaf46301551efeeb54b062f71d2dcf1ffe71fb8
|
|
ab3c06db1aed979847158505f3df1dcea9fd6c2b doc: Release notes for default RBF (Andrew Chow)
61d9149e7804e2cec8fecf4150837344322eb301 rpc: Default rbf enabled (Andrew Chow)
e3c33637bac7db8ae56ab497df10911fad773981 wallet: Enable -walletrbf by default (Andrew Chow)
Pull request description:
The GUI currently opts in to RBF by default, but RPCs do not, and `-walletrbf` is default disabled. This PR makes the default in those two places to also opt in.
The last time this was proposed (#9527), the primary objections were the novelty at the time, the inability to bump transactions, and the gui not having the option to disable rbf. In the 5 years since, RBF usage has steadily grown, with ~27% of txs opting in. The GUI has the option to enable/disable RBF, and is also defaulted to having it enabled. And we have the ability to bump RBF'd transactions in both the RPC and the GUI. So I think it makes sense to finally change the default to always opt in to RBF.
ACKs for top commit:
darosior:
reACK ab3c06db1aed979847158505f3df1dcea9fd6c2b
aureleoules:
ACK ab3c06db1aed979847158505f3df1dcea9fd6c2b.
glozow:
utACK ab3c06db1a
Tree-SHA512: 81b012c5033e270f86a87a6a196ccc549eb54b158eebf88e917cc6621d40d7bdcd1566b602688907dd5d364b95a557b29f97dce869cea512e339588262c027b6
|
|
f608f25313d2867a6abdfc38abdb86da40924cfc Squashed 'src/leveldb/' changes from 330dd6235f..22f1e4a02f (fanquake)
Pull request description:
Replaces #25463 (for master).
Includes:
- https://github.com/bitcoin-core/leveldb-subtree/pull/32
Guix Build (arm64):
```bash
ed41ae2555ae3b638b65d870cef385805e621481831ae992e84645f5c234af63 guix-build-bec911e37ac8/output/arm-linux-gnueabihf/SHA256SUMS.part
7d8237026bfccedee0e56e14d7b89cf2dcb52195b94070dc4b6c3c6970fdc774 guix-build-bec911e37ac8/output/arm-linux-gnueabihf/bitcoin-bec911e37ac8-arm-linux-gnueabihf-debug.tar.gz
1afeff9d70864be66f7ac48d31d1977c7844e2cc117d3f0438fb2f9c6f6a56a4 guix-build-bec911e37ac8/output/arm-linux-gnueabihf/bitcoin-bec911e37ac8-arm-linux-gnueabihf.tar.gz
ab2df265897a0142093b582d3c61e2b17a713d93e478b47c7aa2b0a677295007 guix-build-bec911e37ac8/output/arm64-apple-darwin/SHA256SUMS.part
025a52babdee479800801e951f6fe1fe490e1f5fe8361104eb85e7b0319cdb37 guix-build-bec911e37ac8/output/arm64-apple-darwin/bitcoin-bec911e37ac8-arm64-apple-darwin-unsigned.dmg
7b2992bf5543891b1e6ef4f48c52fc5febc58cc31ccf4307edd27d4d630aa54a guix-build-bec911e37ac8/output/arm64-apple-darwin/bitcoin-bec911e37ac8-arm64-apple-darwin-unsigned.tar.gz
3d93eb009ab6459cdca5fe767795f94ba5e00e5969e44bac19c7a5f344d42030 guix-build-bec911e37ac8/output/arm64-apple-darwin/bitcoin-bec911e37ac8-arm64-apple-darwin.tar.gz
b8c30c5c561c96bc4e280ececc0dd1cd673bc6194591b848903aa6c54c9d37cf guix-build-bec911e37ac8/output/dist-archive/bitcoin-bec911e37ac8.tar.gz
cb2632b9b5ff473e504d3d6244191eb5a852718f8ac8bb1032ba1c65e07d6b3f guix-build-bec911e37ac8/output/powerpc64-linux-gnu/SHA256SUMS.part
26ecc2f42ce37f8bd7ba24bf2f80f493cd1fd45b58409de71c44e2445c291c8c guix-build-bec911e37ac8/output/powerpc64-linux-gnu/bitcoin-bec911e37ac8-powerpc64-linux-gnu-debug.tar.gz
4a83381ea472cc71b4a1c6569483a7e85a5f53065c1633828bf7a75d357b0297 guix-build-bec911e37ac8/output/powerpc64-linux-gnu/bitcoin-bec911e37ac8-powerpc64-linux-gnu.tar.gz
455a9af5e7ee1c2857d87abec29284ae7bb447cf7cb2b3befa2f8e0a0a8cbec6 guix-build-bec911e37ac8/output/powerpc64le-linux-gnu/SHA256SUMS.part
3445f53fd150032ec3c3f324e001b4ecf72728900961a7a7789c32ceb7616617 guix-build-bec911e37ac8/output/powerpc64le-linux-gnu/bitcoin-bec911e37ac8-powerpc64le-linux-gnu-debug.tar.gz
baefeaac88bb4fbf8662c8e1150b043aa2534535a82e828c13e971d2c5fa5cbd guix-build-bec911e37ac8/output/powerpc64le-linux-gnu/bitcoin-bec911e37ac8-powerpc64le-linux-gnu.tar.gz
543484396a47def1636d4e54d4a105c7093265c8896165a4140edec10aeef880 guix-build-bec911e37ac8/output/riscv64-linux-gnu/SHA256SUMS.part
ac1f6e57016703f1319a3ef80014581aee96053e56525b8cd11ada2395496b86 guix-build-bec911e37ac8/output/riscv64-linux-gnu/bitcoin-bec911e37ac8-riscv64-linux-gnu-debug.tar.gz
f9a2b65ed21f777524b078046c84b98239b0fbb92eae8d840bc7a25cab0eec6d guix-build-bec911e37ac8/output/riscv64-linux-gnu/bitcoin-bec911e37ac8-riscv64-linux-gnu.tar.gz
a08ed0ee78ab1c4c815ba8368f1a21d3bd4327ce1104cb3793d63edd2bde1ef4 guix-build-bec911e37ac8/output/x86_64-apple-darwin/SHA256SUMS.part
74059397c6c8f0e899b60415d1aae04f2f7df18b8f39cea9f314e5e0c2e0ede6 guix-build-bec911e37ac8/output/x86_64-apple-darwin/bitcoin-bec911e37ac8-x86_64-apple-darwin-unsigned.dmg
6909ff6f59b78059d505f2b98e6fad63a4e3deb843566061c4cd6e94be1de066 guix-build-bec911e37ac8/output/x86_64-apple-darwin/bitcoin-bec911e37ac8-x86_64-apple-darwin-unsigned.tar.gz
21b45719d927422acc69662108e7255d8cd0b1d832493e70c622c1b1b3a3a314 guix-build-bec911e37ac8/output/x86_64-apple-darwin/bitcoin-bec911e37ac8-x86_64-apple-darwin.tar.gz
b5fdee6fd2dbcdaa6302f388c7fcaa6d130ff91fd5760e19facf6e24c7216ef6 guix-build-bec911e37ac8/output/x86_64-linux-gnu/SHA256SUMS.part
56db24b0b0d2f463c8085a12502977c6d4f4ee5b85484e52522361f54ab3a6aa guix-build-bec911e37ac8/output/x86_64-linux-gnu/bitcoin-bec911e37ac8-x86_64-linux-gnu-debug.tar.gz
96dbe08e2ed0f68fe734dd6f0280c2d22af7b56c84debde424367054f118173f guix-build-bec911e37ac8/output/x86_64-linux-gnu/bitcoin-bec911e37ac8-x86_64-linux-gnu.tar.gz
70ac424b229befb2834a8a02ce27551e3ddde2d438497e8c11a3cedf0cea5d3c guix-build-bec911e37ac8/output/x86_64-w64-mingw32/SHA256SUMS.part
bad144a599b28e8dc0018cc2fd1754543e79df39d651e58f565c197241f2b8cb guix-build-bec911e37ac8/output/x86_64-w64-mingw32/bitcoin-bec911e37ac8-win64-debug.zip
7ce2e72621eb070a8d23b7edbaaccc9f06257b82b9851c1cad4c61f08d2c7451 guix-build-bec911e37ac8/output/x86_64-w64-mingw32/bitcoin-bec911e37ac8-win64-setup-unsigned.exe
5f726ef8b478e3ac90b93cd3ae3c38a0e7bffa5f80306c46a7535518a73251c7 guix-build-bec911e37ac8/output/x86_64-w64-mingw32/bitcoin-bec911e37ac8-win64-unsigned.tar.gz
87cf1a23e948e471ed35c6a518813505c907c58788b55665829e7f12f33bd312 guix-build-bec911e37ac8/output/x86_64-w64-mingw32/bitcoin-bec911e37ac8-win64.zip
```
ACKs for top commit:
hebasto:
ACK bec911e37ac826d55b789428bc07280abab76443, I have reviewed the code and it looks OK, I agree it can be merged.
jarolrod:
ACK bec911e37ac826d55b789428bc07280abab76443
Tree-SHA512: 190381d9489ec6cc52bb9557750925c8574f1344eb6893095e9e31e66a579bd1bc283e8cbfcba52cec3fb072985895f929103b6f5351a23f908bdd0a04b474f1
|