| Age | Commit message (Collapse) | Author |
|---|
|
54011e7aa274bdc1b921440cc8b4623aa1e0d89e refactor: use CWallet const shared pointers when possible (Karl-Johan Alm)
96461989a2de737151bc4fb216221bf49cb53ce6 refactor: const shared_ptrs (Karl-Johan Alm)
Pull request description:
```C++
const std::shared_ptr<CWallet> wallet = x;
```
means we can not do `wallet = y`, but we can totally do `wallet->DestructiveOperation()`, contrary to what that line looks like.
This PR
* introduces a new convention: always use const shared pointers to `CWallet`s (even when we mutate the pointed-to thing)
* uses `const shared_ptr<const CWallet>` everywhere where wallets are not modified
In the future, this should preferably apply to all shared pointers, not limited to just `CWallet`s.
Both of these serve the same purpose: to dispell the misconception that `const shared_ptr<X>` immutates `X`. It doesn't, and it's dangerous to leave this misconception as is, for obvious reasons.
ACKs for top commit:
theStack:
re-ACK 54011e7aa274bdc1b921440cc8b4623aa1e0d89e
Tree-SHA512: 3bf4062fc821751be30770c6b4ead10a016847970f155a0a5156f304347d221b9830840030c2fbfba8cd1e282f4eda45f5b4107fe6df8138afdcb6c2e95a2836
|
|
fa8fef6ef2287cd36ae14fbf10e852ddef7e62ac doc: Fix CWalletTx::Confirmation doc (MarcoFalke)
Pull request description:
Follow-up to:
* commit 700c42b85d20e624bef4228eef062c93084efab5, which replaced pIndex
with block_hash in AddToWalletIfInvolvingMe.
* commit 9700fcb47feca9d78e005b8d18b41148c8f6b25f, which replaced
posInBlock with confirm.nIndex.
ACKs for top commit:
laanwj:
Code review ACK fa8fef6ef2287cd36ae14fbf10e852ddef7e62ac
ryanofsky:
Code review ACK fa8fef6ef2287cd36ae14fbf10e852ddef7e62ac
Tree-SHA512: e7643b8e9200b8ebab3eda30435ad77006568a03476006013c9ac42c826c84e42e29bcdefc6f443fe4d55e76e903bfed5aa7a51f831665001c204634b6f06508
|
|
da791c7f66243080177f92ce5f38c49305da63dc wallet: Use PACKAGE_NAME to mention our software (Hennadii Stepanov)
Pull request description:
This PR replaces "bitcoin" and "bitcoind" with `PACKAGE_NAME` in wallet log and error messages.
ACKs for top commit:
jonatack:
ACK da791c7f66243080177f92ce5f38c49305da63dc
lsilva01:
Tested ACK da791c7 on Ubuntu 20.04.
brunoerg:
tACK da791c7f66243080177f92ce5f38c49305da63dc
stratospher:
Tested ACK da791c7
shaavan:
ACK da791c7f66243080177f92ce5f38c49305da63dc
Tree-SHA512: c613446d9c8c3f85e6f5fec77382c9bc17746a853c89e72e1a3a79bf355d7bd9e455bbde8f9e02a894d225a67029c732cdc68ec8c58ac8237dde27d39dae8be7
|
|
While const shared_ptr<X> gives us an immutable shared pointer to a mutable X (we can't set it to some other X later), shared_ptr<const X> gives us a shared pointer to an immutable X. Importantly, we can recast shared_ptr<X> into shared_ptr<const X>, but not the other way around. We do this for two reasons: because it makes the code safer to guarantee the wallet is not modified, and because it further dispells the misconception that const shared_ptr<X> gives immutability to X.
|
|
Introduce convention to use const shared pointers everywhere, unless the shared pointer is modified at some point, which it very rarely is.
We want this convention, as it helps alleviate the misconception that a const shared pointer somehow results in a pointer to an immutable object, which is false.
|
|
Default to SQLiteDatabase instead of BerkeleyDatabase for
CreateDummyWalletDatabase. Most tests already use descriptor wallets and
the mock db doesn't really matter for tests. The tests where it does
matter will make the db directly.
|
|
Instead of using AddToWallet so that making a COutput will work,
directly add the transaction into wallet.mapWallet. This bypasses many
checks that AddToWallet will do which are pointless and just slow down
this test.
|
|
TransactionDescriptionString(), add coverage
66f6efc70a72cc1613906fd3c10281f9af0ba0db rpc: improve TransactionDescriptionString() "generated" help (Jon Atack)
296cfa312fd9ce19f1f820aeafa37d87764ad21d test: add listtransactions/listsinceblock "trusted" coverage (Jon Atack)
d95913fc432f0fde9dec743884b14c5df83727af rpc: fix "trusted" description in TransactionDescriptionString (Jon Atack)
Pull request description:
The RPC gettransaction, listtransactions, and listsinceblock helps returned by `TransactionDescriptionString()` inform the user that the `trusted` boolean field is only present if the transaction is trusted and safe to spend from.
The field is in fact returned by `WalletTxToJSON()` when the transaction has 0 confirmations (or negative confirmations, if conflicted), and it can be true or false.
This patch fixes the help, adds test coverage, and touches up the help for the neighboring `generate` field.
ACKs for top commit:
rajarshimaitra:
tACK https://github.com/bitcoin/bitcoin/pull/23139/commits/66f6efc70a72cc1613906fd3c10281f9af0ba0db
theStack:
Tested ACK 66f6efc70a72cc1613906fd3c10281f9af0ba0db
Tree-SHA512: 4c2127765b82780e07bbdbf519d27163d414d9f15598e01e02210f210e6009be344c84951d7274e747b1386991d4c3b082cd25aebe885fb8cf0b92d57178f68e
|
|
from some wallet tests
2d2edc1248a2e49636409b07448676e5bfe44956 tests: Use Descriptor wallets for generic wallet tests (Andrew Chow)
99516285b7cf2664563712d95d95f54e1985c0c2 tests: Use legacy change type in subtract fee from outputs test (Andrew Chow)
dcd6eeb64adb2b532f5003cbb86ba65b3c08a87b tests: Use descriptors in psbt_wallet_tests (Andrew Chow)
4b1588c6bd96743b333cc291e19a9fc76dc8cdf1 tests: Use DescriptorScriptPubKeyMan in coinselector_tests (Andrew Chow)
811319fea4295bfff05c23c0dcab1e24c85e8544 tests, gui: Use DescriptorScriptPubKeyMan in GUI tests (Andrew Chow)
9bf02438727e1052c69d906252fc2a451c923409 bench: Use DescriptorScriptPubKeyMan for wallet things (Andrew Chow)
5e54aa9b90c5d4d472be47a7fca969c5e7b92e88 bench: remove global testWallet from CoinSelection benchmark (Andrew Chow)
a5595b1320d0ebd2c60833286799ee42108a7c01 tests: Remove global vCoins and testWallet from coinselector_tests (Andrew Chow)
Pull request description:
Currently, various tests use `LegacyScriptPubKeyMan` because it was convenient for the refactor that introduced the `ScriptPubKeyMan` interface. However, with the legacy wallet slated to be removed, these tests should not continue to use `LegacyScriptPubKeyMan` as they are not testing any specific legacy wallet behavior. These tests are changed to use `DescriptorScriptPubKeyMan`s.
Some of the coin selection tests and benchmarks had a global `testWallet`, but this seemed to cause some issues with ensuring that descriptors were set up in that wallet for each test. Those have been restructured to not have any global variables that may be modified between tests.
The tests which test specific legacy wallet behavior remain unchanged.
ACKs for top commit:
laanwj:
Code review ACK 2d2edc1248a2e49636409b07448676e5bfe44956
brunoerg:
tACK 2d2edc1248a2e49636409b07448676e5bfe44956
Tree-SHA512: 6d60e5978e822d48e46cfc0dae4635fcb1939f21ea9d84eb72e36112e925554b7ee8f932c7ed0c4881b6566c6c19260bec346abdff1956ca9f300b30fb4e2dd1
|
|
default-ctored `external_spk_managers` entry
6911ab95f19d2b1f60f2d0b2f3961fa6639d4f31 wallet: fix segfault by avoiding invalid default-ctored `external_spk_managers` entry (Sebastian Falbesoner)
Pull request description:
Fixes #23321 (bug reported by Josef Vondrlik (josef-v)).
In the method `CWallet::LoadActiveScriptPubKeyMan`, the map `external_spk_managers` (or `internal_spk_managers`, if parameter `internal` is false) is accessed via std::map::operator[], which means that a default-ctored entry is created with a null-pointer as value, if the key doesn't exist. As soon as this value is dereferenced, a segmentation fault occurs, e.g. in `CWallet::KeypoolCountExternalKeys`.
The bevaviour can be reproduced by the following steps (starting with empty regtest datadir):
```
$ ./src/bitcoind -regtest -daemon
$ ./src/bitcoin-cli -regtest -named createwallet_name=wallet descriptors=true blank=true
$ cat regtest-descriptors.txt
[
{
"desc": "tr([e4445899/49'/1'/0']tprv8ZgxMBicQKsPd8jCeBWsYLEoWxbVgzJDatJ7XkwQ6G3uF4FsHuaziHQ5JZAW4K515nj6kVVwPaNWZSMEcR7aFCwL4tQqTcaoprMKTTtm6Zg/1/*)#mr3llm7f",
"timestamp": 1634652324,
"active": true,
"internal": true,
"range": [
0,
999
],
"next": 0
}
]
$ ./src/bitcoin-cli -regtest importdescriptors "$(cat regtest-descriptors.txt)"
[
{
"success": true
}
]
$ ./src/bitcoin-cli -regtest getwalletinfo
error: timeout on transient error: Could not connect to the server 127.0.0.1:18443 (error code 1 - "EOF reached")
```
ACKs for top commit:
achow101:
Code Review ACK 6911ab95f19d2b1f60f2d0b2f3961fa6639d4f31
lsilva01:
Tested ACK 6911ab9 on Ubuntu 20.04.
instagibbs:
ACK 6911ab95f19d2b1f60f2d0b2f3961fa6639d4f31
Tree-SHA512: 76aa96847cf2739413fb68fb902afef0b3ab9381178dd62fb0abac69f853f1f6523d73c60e610375b9a7730f275eda9162503b89f5be6e6e349a8d047b59c8dc
|
|
9c1052a5218e191fd23c0d9fc06f2fca34b03411 wallet: Default new wallets to descriptor wallets (Andrew Chow)
f19ad404631010a5e2dac2c7cbecd057b005fe2a rpc, wallet: Descriptor wallets are no longer experimental (Andrew Chow)
Pull request description:
Changes the default wallet type from legacy to descriptors. Descriptor wallets will now by the default type. Additionally, descriptor wallets will no longer be marked as experimental.
This follows the timeline proposed in #20160
ACKs for top commit:
lsilva01:
Tested ACK https://github.com/bitcoin/bitcoin/pull/23002/commits/9c1052a5218e191fd23c0d9fc06f2fca34b03411 on Ubuntu 20.04
prayank23:
tACK https://github.com/bitcoin/bitcoin/pull/23002/commits/9c1052a5218e191fd23c0d9fc06f2fca34b03411
meshcollider:
Code review ACK 9c1052a5218e191fd23c0d9fc06f2fca34b03411
Tree-SHA512: 834e6fec88e0c18673af7ebe135bd5333694d1be502164eb93a90e3e76c27974165aa4e59426945100c88e4eca07356e16886ef5b05cf789683ecb23fc71a12a
|
|
Follow-up to:
* commit 700c42b85d20e624bef4228eef062c93084efab5, which replaced pIndex
with block_hash in AddToWalletIfInvolvingMe.
* commit 9700fcb47feca9d78e005b8d18b41148c8f6b25f, which replaced
posInBlock with confirm.nIndex.
|
|
`external_spk_managers` entry
In the method `CWallet::LoadActiveScriptPubKeyMan`, the map
`external_spk_managers` (or `internal_spk_managers`, if parameter
`internal` is false) is accessed via std::map::operator[], which means
that a default-ctored entry is created with a null-pointer as value, if
the key doesn't exist. As soon as this value is dereferenced, a
segmentation fault occurs, e.g. in `CWallet::KeypoolCountExternalKeys`.
The bevaviour can be reproduced by the following steps (starting with empty regtest datadir):
$ ./src/bitcoind -regtest -daemon
$ ./src/bitcoin-cli -regtest -named createwallet_name=wallet descriptors=true blank=true
$ cat regtest-descriptors.txt
[
{
"desc": "tr([e4445899/49'/1'/0']tprv8ZgxMBicQKsPd8jCeBWsYLEoWxbVgzJDatJ7XkwQ6G3uF4FsHuaziHQ5JZAW4K515nj6kVVwPaNWZSMEcR7aFCwL4tQqTcaoprMKTTtm6Zg/1/*)#mr3llm7f",
"timestamp": 1634652324,
"active": true,
"internal": true,
"range": [
0,
999
],
"next": 0
}
]
$ ./src/bitcoin-cli -regtest importdescriptors "$(cat regtest-descriptors.txt)"
[
{
"success": true
}
]
$ ./src/bitcoin-cli -regtest getwalletinfo
error: timeout on transient error: Could not connect to the server 127.0.0.1:18443 (error code 1 - "EOF reached")
Bug reported by Josef Vondrlik (josef-v).
|
|
|
|
::ChainActive()
a0efe529e4fd053b890450413b9ca5e1bcd8f2c2 Fix outdated comments referring to ::ChainActive() (Samuel Dobson)
Pull request description:
After #21866 there are a few outdated comments referring to `::ChainActive()`, which should instead refer to `ChainstateManager::ActiveChain()`.
ACKs for top commit:
jamesob:
ACK https://github.com/bitcoin/bitcoin/pull/23258/commits/a0efe529e4fd053b890450413b9ca5e1bcd8f2c2
Tree-SHA512: 80da19c105ed29ac247e6df4c8e916c3bf3f37230b63f07302114eef9c115add673e9649f0bbe237295be0c6da7b1030b5b93e14daf6768f17ce5de7cf2c9ff2
|
|
For the generic wallet tests, make DescriptorScriptPubKeyMans. There are
still some wallet tests that test legacy wallet things. Those remain
unchanged.
|
|
The subtract fee from outputs assumes that the leftover input amount
will be dropped to fees. However this only happens if that amount is
less than the cost of change. In the event that it is higher than the
cost of change, the leftover amount will actually become a change
output. To avoid this scenario, force a change type which has a high
cost of change.
|
|
|
|
|
|
To avoid issues with test data leaking across tests cases, the global
vCoins and testWallet are removed from coinselector_tests and all of the
relevant functions reworked to not need them.
|
|
fs::path(std::string) constructor and fs::path::string() method
6544ea5035268025207d2402db2f7d90fde947a6 refactor: Block unsafe fs::path std::string conversion calls (Russell Yanofsky)
b39a477ec69a51b2016d3a8c70c0c77670f87f2b refactor: Add fs::PathToString, fs::PathFromString, u8string, u8path functions (Russell Yanofsky)
Pull request description:
The `fs::path` class has a `std::string` constructor which will implicitly convert from strings. Implicit conversions like this are not great in general because they can hide complexity and inefficiencies in the code, but this case is especially bad, because after the transition from `boost::filesystem` to `std::filesystem` in #20744 the behavior of this constructor on windows will be more complicated and can mangle path strings. The `fs::path` class also has a `.string()` method which is inverse of the constructor and has the same problems.
Fix this by replacing the unsafe method calls with `PathToString` and `PathFromString` function calls, and by forbidding unsafe method calls in the future.
ACKs for top commit:
kiminuo:
ACK 6544ea5035268025207d2402db2f7d90fde947a6
laanwj:
Code review ACK 6544ea5035268025207d2402db2f7d90fde947a6
hebasto:
re-ACK 6544ea5035268025207d2402db2f7d90fde947a6, only added `fsbridge_stem` test case, updated comment, and rebased since my [previous](https://github.com/bitcoin/bitcoin/pull/22937#pullrequestreview-765503126) review. Verified with the following command:
Tree-SHA512: c36324740eb4ee55151146626166c00d5ccc4b6f3df777e75c112bcb4d1db436c1d9cc8c29a1e7fb96051457d317961ab42e6c380c3be2771d135771b2b49fa0
|
|
when upgrading non-HD to HD
6531599f422524fbbcc43816121e7536cf79d66c test: Add check that newkeypool flushes change addresses too (Samuel Dobson)
84fa19c77a2c8d0d01add2daf18b42af07c17710 Add release notes for keypool flush changes (Samuel Dobson)
f9603ee4e05d7f0bd7d81f5cf24168c1aec8e5b0 Add test for flushing keypool with newkeypool (Samuel Dobson)
6f6f7bb36c492fa76aeda6513be58ca822ea1968 Make legacy wallet upgrades from non-HD to HD always flush the keypool (Samuel Dobson)
2434b1078147e71b09c4c1bf0b7ce3f6729a7713 Fix outdated keypool size default (Samuel Dobson)
22cc797ca5c1e70a4afb8e43f6917b4c9fe74e20 Add newkeypool RPC to flush the keypool (Samuel Dobson)
Pull request description:
This PR makes two main changes:
1) Adds a new RPC `newkeypool` which will entirely flush and refill the keypool.
2) When upgradewallet is called on old, non-HD wallets upgrading them to HD, we now always flush the keypool and generate a new one, to immediately start using the HD generated keys.
This PR is motivated by a number of users with old, pre-compressed-key wallets upgrading them and being confused about why they still can't generate p2sh-segwit or bech32 addresses -- this is due to uncompressed keys remaining in the keypool post-upgrade and being illegal in these newer address formats. There is currently no easy way to flush the keypool other than to call `getnewaddress` a hundred/thousand times or an ugly hack of using a `sethdseed` call.
ACKs for top commit:
laanwj:
re-ACK 6531599f422524fbbcc43816121e7536cf79d66c
meshcollider:
Added new commit 6531599f422524fbbcc43816121e7536cf79d66c to avoid invalidating previous ACKs.
instagibbs:
ACK https://github.com/bitcoin/bitcoin/pull/23093/commits/6531599f422524fbbcc43816121e7536cf79d66c
Tree-SHA512: 50c79c5d42dd27ab0ecdbfdc4071fdaa1b2dbb2f9195ed325b007106ff19226419ce57fe5b1539c0c24101b12f5e034bbcfb7bbb0451b766cb1071295383d774
|
|
|
|
43568782c23185a0599a6e60d61db4716da1cda1 External input fund support cleanups (Gregory Sanders)
Pull request description:
Minor cleanups to https://github.com/bitcoin/bitcoin/pull/17211
ACKs for top commit:
achow101:
ACK 43568782c23185a0599a6e60d61db4716da1cda1
meshcollider:
utACK 43568782c23185a0599a6e60d61db4716da1cda1
benthecarman:
ACK 43568782c23185a0599a6e60d61db4716da1cda1
Tree-SHA512: 865f8a3804f8c0027f5393a0539041158166a919378f2c3bc99b936843eee2329372bcc2af888fa62babfa5f6baf4f13d4cfef7b4e26a7265a82a908f9719ad6
|
|
fafff132cf4e5c2950d28f63cb4320236d1a5495 doc: Extract FundTxDoc (MarcoFalke)
Pull request description:
No need to duplicate the documentation for the same field(s) three times.
Fix that by de-duplicating it for the fields: conf_target, estimate_mode, replaceable, and solving_data.
Can be reviewed with `--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`.
ACKs for top commit:
fanquake:
ACK fafff132cf4e5c2950d28f63cb4320236d1a5495
Tree-SHA512: 098ddad3904b80b24c9e7b57ca8e807a6ccc3899eac2c9986d71ba3873c2b580bbb95f2fdfbf94b2db02f81c7b0ebf438a90324c23389b7b968ca85ae8475373
|
|
Synchronize error checking for external inputs
Rename external output Select to SelectExternal
Const FundTransaction variables
|
|
There is no change in behavior. This just helps prepare for the
transition from boost::filesystem to std::filesystem by avoiding calls
to methods which will be unsafe after the transaction to std::filesystem
to due lack of a boost::filesystem::path::imbue equivalent and inability
to set a predictable locale.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Co-authored-by: Kiminuo <kiminuo@protonmail.com>
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
|
|
9d0379cea6c164610d05287ae6dd4e66f35b92b3 consensus: use <cstdint> over <stdint.h> in amount.h (fanquake)
863e52fe63a67fa020fb1ef527b9095a35ab77a5 consensus: make COIN & MAX_MONEY constexpr (fanquake)
d09071da5bc997f2de1f55ca7a9babc3d7619329 [MOVEONLY] consensus: move amount.h into consensus (fanquake)
Pull request description:
A first step (of a few) towards some source code reorganization, as well as making libbitcoinconsensus slightly more self contained.
Related to #15732.
ACKs for top commit:
MarcoFalke:
concept ACK 9d0379cea6c164610d05287ae6dd4e66f35b92b 🏝
Tree-SHA512: 97fc79262dcb8c00996852a288fee69ddf8398ae2c95700bba5b326f1f38ffcfaf8fa66e29d0cb446d9b3f4e608a96525fae0c2ad9cd531ad98ad2a4a687cd6a
|
|
For the fields: conf_target, estimate_mode, replaceable, and solving_data.
|
|
with locale-independent std::from_chars(…) (C++17)
4343f114cc661cf031ec915538c11b9b030e2e15 Replace use of locale dependent atoi(…) with locale-independent std::from_chars(…) (C++17) (practicalswift)
Pull request description:
Replace use of locale dependent `atoi(…)` with locale-independent `std::from_chars(…)` (C++17).
ACKs for top commit:
laanwj:
Code review ACK 4343f114cc661cf031ec915538c11b9b030e2e15
jonatack:
Code review ACK 4343f114cc661cf031ec915538c11b9b030e2e15
Tree-SHA512: e4909da282b6cefc5ca34e13b02cc489af56cab339a77ae5c35ac9ef355d9b941b129a2bfddc1b37426b11c79a21c8b729fbb5255e6d9eaa344406b18b825494
|
|
walletcreatefundedpsbt to take external inputs
928af61cdb2c4de1c3d10e6fda13bbba5ca0bba9 allow send rpc take external inputs and solving data (Andrew Chow)
e39b5a5e7aa4d015257565ca79dc7b1f7a65e074 Tests for funding with external inputs (Andrew Chow)
38f5642cccf2b6708e58f5e2af5ecdcf752e61ec allow fundtx rpcs to work with external inputs (Andrew Chow)
d5cfb864ae16da62399bc97ab1ed54d32cf0cce9 Allow Coin Selection be able to take external inputs (Andrew Chow)
a00eb388e8046fe105666445dff6c91e8f8664cb Allow CInputCoin to also be constructed with COutPoint and CTxOut (Andrew Chow)
Pull request description:
Currently `fundrawtransaction` and `walletcreatefundedpsbt` both do not allow external inputs as the wallet does not have the information necessary to estimate their fees.
This PR adds an additional argument to both those RPCs which allows the user to specify solving data. This way, the wallet can use that solving data to estimate the size of those inputs. The solving data can be public keys, scripts, or descriptors.
ACKs for top commit:
prayank23:
reACK https://github.com/bitcoin/bitcoin/commit/928af61cdb2c4de1c3d10e6fda13bbba5ca0bba9
meshcollider:
Re-utACK 928af61cdb2c4de1c3d10e6fda13bbba5ca0bba9
instagibbs:
crACK 928af61cdb2c4de1c3d10e6fda13bbba5ca0bba9
yanmaani:
utACK 928af61.
Tree-SHA512: bc7a6ef8961a7f4971ea5985d75e2d6dc50c2a90b44c664a1c4b0f1be5c1c97823516358fdaab35771a4701dbefc0862127b1d0d4bfd02b4f20d2befa4434700
|
|
|
|
|
|
0ab4c3b27265401c59e40adc494041927dc9dbe3 Return false on corrupt tx rather than asserting (Samuel Dobson)
Pull request description:
Takes up #19793
Rather than asserting, we log an error and return CORRUPT so that the user is informed. This type of error isn't critical so it isn't worth `assert`ing.
ACKs for top commit:
achow101:
ACK 0ab4c3b27265401c59e40adc494041927dc9dbe3
laanwj:
Code review ACK 0ab4c3b27265401c59e40adc494041927dc9dbe3
ryanofsky:
Code review ACK 0ab4c3b27265401c59e40adc494041927dc9dbe3. There may be room for more improvements later like better error messages or easier recovery options, but changing from an assert to an error seems like a clear improvement, and this seems to avoid all the pitfalls of the last PR that tried this.
Tree-SHA512: 4a1a412e7c473d176c4e09123b85f390a6b0ea195e78d28ebd50b13814b7852f8225a172511a2efb6affb555b11bd4e667c19eb8c78b060c5444b62f0fae5f7a
|
|
-BEGIN VERIFY SCRIPT-
git grep -l 'RESCAN_REQUIRED' src | xargs sed -i 's/RESCAN_REQUIRED/NEED_RESCAN/g'
-END VERIFY SCRIPT-
|
|
dc3ec74d67abc85e8f724648f93efdd097e6f783 Add rescan removal release note (Samuel Dobson)
bccd1d942d971e70e7a0f4f5628e1b74b3ac15e0 Remove -rescan startup parameter (Samuel Dobson)
f963b0fa8cdd5223feb828c5faf6c57bc4107c8a Corrupt wallet tx shouldn't trigger rescan of all wallets (Samuel Dobson)
6c006495ef07f163d0734ec35d3cd1589a4aae9d Remove outdated dummy wallet -salvagewallet arg (Samuel Dobson)
Pull request description:
Remove the `-rescan` startup parameter.
Rescans can be run with the `rescanblockchain` RPC.
Rescans are still done on wallet-load if needed due to corruption, for example.
ACKs for top commit:
achow101:
ACK dc3ec74d67abc85e8f724648f93efdd097e6f783
laanwj:
re-ACK dc3ec74d67abc85e8f724648f93efdd097e6f783
Tree-SHA512: 608360d0e7d73737fd3ef408b01b33d97a75eebccd70c6d1b47a32fecb99b9105b520b111b225beb10611c09aa840a2b6d2b6e6e54be5d0362829e757289de5c
|
|
std::from_chars(…) (C++17)
test: Add test cases for LocaleIndependentAtoi
fuzz: Assert legacy atoi(s) == LocaleIndependentAtoi<int>(s)
fuzz: Assert legacy atoi64(s) == LocaleIndependentAtoi<int64_t>(s)
|
|
multiple lines in the log file
2222c04e1b9960030cb590c789a0d2375add4544 log: Adjust coin selection log string (MarcoFalke)
fa6c1e850f3a96f884ba8a635b72d3abea1f4e56 test: Fix typos in tests (MarcoFalke)
faeae2980fa2493391cdced20950a991e28cf47d log: Avoid broken DEBUG_LOCKORDER log (MarcoFalke)
faffaa85cde32b621f598a8ea8dceae34f33f021 log: Avoid broken SELECTCOINS log (MarcoFalke)
Pull request description:
Follow up to commit d8b4b3077fd20c90b635eff1dd240bdad9725027
ACKs for top commit:
laanwj:
re-ACK 2222c04e1b9960030cb590c789a0d2375add4544
practicalswift:
cr ACK 2222c04e1b9960030cb590c789a0d2375add4544
Tree-SHA512: e0daf76815a1b7c4898ceffedeaf7ede093223abf709874f9a0d78c8e41551c14e8b56d055c8fdf06ec698df64e67dfc168bbd8716131b23648d1d1294fa6636
|
|
90be29c5b52e68b5de8a3282cd83172fbf9acf1b wallet: enable SQLite extended result codes (Sebastian Falbesoner)
Pull request description:
With this change, we get more fine-grained error messages if something goes wrong in the course of communicating with the SQLite database. To pick some random examples, the error codes SQLITE_IOERR_NOMEM, SQLITE_IOERR_CORRUPTFS or SQLITE_IOERR_FSYNC are way more specific than just a plain SQLITE_IOERR, and the corresponding error messages generated by sqlite3_errstr() will hence give a better hint to the user (or also to the developers, if an error report is sent) what the cause for a failure is.
See the SQLite documentation
https://www.sqlite.org/c3ref/extended_result_codes.html
https://www.sqlite.org/c3ref/c_abort_rollback.html
> In its default configuration, SQLite API routines return one of 30 integer result codes. However, experience has shown that many of these result codes are too coarse-grained. They do not provide as much information about problems as programmers might like. In an effort to address this, newer versions of SQLite (version 3.3.8 2006-10-09 and later) include support for additional result codes that provide more detailed information about errors.
ACKs for top commit:
Sjors:
utACK 90be29c
achow101:
ACK 90be29c5b52e68b5de8a3282cd83172fbf9acf1b
laanwj:
Code review ACK 90be29c5b52e68b5de8a3282cd83172fbf9acf1b
Tree-SHA512: 2b7a60860c206f2b5f8ff9d4a7698efdee897c9ad024621b8fd165b841c20746d9780da3cf46aaf448a777e229a5b3cdf3a4792e8ef82cda9c5d46e354a9a598
|
|
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: Andrew Chow <achow101-github@achow101.com>
|
|
Move amount.h to consensus/amount.h.
Renames, adds missing and removes uneeded includes.
|
|
|
|
|
|
|
|
|
|
|
|
The helps for RPCs gettransaction, listtransactions, and
listsinceblock returned by TransactionDescriptionString()
state that the "trusted" boolean field is only present if the
transaction is trusted and safe to spend from.
The "trusted" boolean field is in fact returned by
WalletTxToJSON() when the transaction has 0 confirmations,
or negative confirmations, if conflicted, and it can be
true or false.
This commit updates TransactionDescriptionString() to a
more accurate description for "trusted" and updates the
existing line of test coverage to fail more helpfully.
|
|
Replace the outdated function name with words from the English language.
Logging the function name can be toggled with -logsourcelocations.
|
|
during rescanning process.
240ea294d5e899a906f213f039b21e94c24d6018 doc: update doxygen documention of ComputeTimeSmart() and AddToWalletIfInvolvingMe() regarding rescanning_old_block parameter (BitcoinTsunami)
d6eb39af21810bf1c3bdce0ef2212c1ad6597bcd test: add functional test to check transaction time determination during block rescanning (BitcoinTsunami)
07b44f16e71b9df10dfac7f32f92997938f7e7aa wallet: fix ComputeTimeSmart algorithm to use blocktime during old block rescanning (BitcoinTsunami)
Pull request description:
The function ComputeTimeSmart in wallet.cpp assume that transaction are discovered in the right order.
Moreover the 'smarttime' determination algorithm is coded with realtime scenario in mind and not rescanning of old block.
The functional test demonstrate that if the user import a wallet, then rescan only recent history, and then rescan the entire history, the older transaction discovered would have an incorrect time determination.
In the context of rescanning old block, the only time value that as a meaning is the blocktime.
That's why I've fixed the problem with a simple separation between rescanning of old block and realtime time determination. The fix is written to have no impact on every realtime scenario and only impact the behaviour during a rescanning process.
This PR Fixes #20181.
To be fair, I don't think that this bug could be triggered with the wallet GUI, because it always proceed with a proper rescan.
But RPC API provide the possibility to trigger it. I've discovered it, because Specter desktop v0.10.0 was impacted. (https://github.com/cryptoadvance/specter-desktop/issues/680).
ACKs for top commit:
jonatack:
ACK 240ea294d5e899a906f213f039b21e94c24d6018 per `git diff b92d552 240ea29`, re-verified rebase to latest master + debug build clean + new test passes on the branch and fails on master, only change since my review a few hours ago is incorporation of latest review suggestions
meshcollider:
re-utACK 240ea294d5e899a906f213f039b21e94c24d6018
Tree-SHA512: 514b02e41d011ddfa325f5e8080b93800e1ea4ed5853fa420670a6ac700e8b463000dbea65f8ced8565cfb950c7f51b69154034dcb111e67aca3b964a0061494
|
|
corresponding code/logic
43cd6b8af9d613ca033800c5cd8524c3f77e13ec doc: add release notes for removal of the -deprecatedrpc=addresses flag (Michael Dietz)
2b1fdc2c6ce1d0b0e51a3f107e23443c142d57af refactor: minor styling, prefer snake case and same line if (Michael Dietz)
d64deac7b823a0eba97ab3a3686054eefe330d3c refactor: share logic between ScriptPubKeyToUniv and ScriptToUniv (Michael Dietz)
8721638daa8502c7f8de5ae24a9393d7290a2ce5 rpc: remove deprecated addresses and reqSigs from rpc outputs (Michael Dietz)
Pull request description:
Resolves #21797 now that we've branched-off to v23 ("addresses" and "reqSigs" deprecated) "ExtractDestinations" should be removed.
`-deprecatedrpc=addresses` was initially added in this PR #20286 (which resolved the original issue #20102).
Some chunks of code and logic are no longer used/necessary with the removal of this, and therefore some minor refactoring is done in this PR as well (separated commits)
ACKs for top commit:
MarcoFalke:
re-ACK 43cd6b8af9d613ca033800c5cd8524c3f77e13ec 🐉
meshcollider:
Code review ACK 43cd6b8af9d613ca033800c5cd8524c3f77e13ec
jonatack:
ACK 43cd6b8af9d613ca033800c5cd8524c3f77e13ec per `git range-diff a9d0cec 92dc5e9 43cd6b8`, also rebased to latest master, debug built + quick re-review of each commit to bring back context, and ran tests locally at the final commit
Tree-SHA512: fba83495e396d3c06f0dcf49292f14f4aa6b68fa758f0503941fade1a6e7271cda8378e2734af1faea550d1b43c85a36c52ebcc9dec0732936f9233b4b97901c
|
