Age | Commit message (Collapse) | Author |
|
SubmitMemoryPoolAndRelay()
c8b53c3beafa289dcdbd8c2ee9f957bdeca79cbc [wallet] Restore confirmed/conflicted tx check in SubmitMemoryPoolAndRelay() (John Newbery)
214c4ecb9ab306627a08abb35cf82c73f10ec627 [wallet] restore coinbase check in SubmitMemoryPoolAndRelay() (John Newbery)
Pull request description:
These checks don't change mempool acceptance/relay behaviour, but reduce log spam.
ACKs for top commit:
MarcoFalke:
ACK c8b53c3beafa289dcdbd8c2ee9f957bdeca79cbc (non-doc changes are mostly a git revert 8753f5652b4710e66b50ce87788bf6f33619b75a)
ariard:
utACK c8b53c3
Tree-SHA512: f928573ad68d2f70ac69a84b57f352d255dccd1942097cc664f130fcbdcdd7364bc52c43b9157e65ebbaaebbe93586c6e8386f24361b27478e0a23a445677672
|
|
Restores the confirmed/conflicted tx check removed in
8753f5652b4710e66b50ce87788bf6f33619b75a. There should be no external
behaviour change (these txs would not get accepted to the mempool
anyway), but not having the check in the wallet causes log spam.
Also adds a comment to ResentWalletTransactions() that
confirmed/conflicted tx check is done in SubmitMemoryPoolAndRelay().
|
|
RPC server starts in warmup mode, it can't
process yet calls, then follows connection manager
initialization and finally RPC server get out of
warmup mode. RPC calls shouldn't be able to get
P2P disabled errors because once we initialize
g_connman it's not unset until shutdown, after
RPC server has been stopped.
|
|
This check doesn't change mempool acceptance/relay behaviour, but reduces log spam.
|
|
e6f649cb2c07bf55d9214c2876619c56f1d6fe30 test: Make tests arg type specific (Hennadii Stepanov)
b70cc5d73357ea11296f3b6bb81193ba1101e73b Revamp option negating policy (Hennadii Stepanov)
db08edb3038a085d3dbce7bb4ec3c1d9b9a5b281 Replace IsArgKnown() with FlagsOfKnownArg() (Hennadii Stepanov)
dde80c272ae584410532f48d23866d7d8581a1cc Use ArgsManager::NETWORK_ONLY flag (Hennadii Stepanov)
9a12733508e47f558959f1b0ed9937bc3eff8962 Remove unused m_debug_only member from Arg struct (Hennadii Stepanov)
fb4b9f9e3b433d8848832e2c2686cf7b1f212a5e scripted-diff: Use ArgsManager::DEBUG_ONLY flag (Hennadii Stepanov)
1b4b9422cad28d1bead24ff5fd472536954cfaf9 scripted-diff: Use Flags enum in AddArg() (Hennadii Stepanov)
265c1b58d89b7b6fb30468ba402d7f75cc59a510 Add Flags enum to ArgsManager (Hennadii Stepanov)
e0d187dfeb18b026de22bd7960b2a50c2b958e1a Refactor InterpretNegatedOption() function (Hennadii Stepanov)
e0e18a1017fa3dc5d6ebeda6ec35c4263327d17c refactoring: Check IsArgKnown() early (Hennadii Stepanov)
Pull request description:
This PR adds the `Flags` enum to the `ArgsManager` class. Also the `m_flags` member is added to the `Arg` struct. Flags denote an allowed type of an arg value and special hints.
This PR is only a refactoring and does not change behavior.
ACKs for top commit:
jamesob:
ACK https://github.com/bitcoin/bitcoin/pull/16097/commits/e6f649cb2c07bf55d9214c2876619c56f1d6fe30
MarcoFalke:
ACK e6f649cb2c07bf55d9214c2876619c56f1d6fe30 thanks for adding types to the command line options
Tree-SHA512: b867f8a9cbce2d2473c293d534af662d8cd5be15060ff0682e97af678974bdaac35e8bc6328ccba32f105034bcd38f169b92a6fb67798667891ce14d5d2a2dea
|
|
higher method
fb62f128bbfd8c6cd72ea8e23331a4bae23883ab Tidy up BroadcastTransaction() (John Newbery)
b8eecf8e79dad92ff07b851b1b29c2a66546bbc1 Remove unused submitToMemoryPool and relayTransactions Chain interfaces (Antoine Riard)
8753f5652b4710e66b50ce87788bf6f33619b75a Remove duplicate checks in SubmitMemoryPoolAndRelay (Antoine Riard)
611291c198eb2be9bf1aea1bf9b2187b18bdb3aa Introduce CWalletTx::SubmitMemoryPoolAndRelay (Antoine Riard)
8c8aa19b4b4fa56cd359092ef099bcfc7b26c334 Add BroadcastTransaction utility usage in Chain interface (Antoine Riard)
Pull request description:
Remove CWalletTx::AcceptToMemoryPool
Replace CWalletTx::RelayWalletTransaction by SubmitMemoryPoolAndRelay
Add a relay flag to broadcastTransaction because wasn't sure of ReacceptWalletTransactions semantic.
Obviously, working on implementing https://github.com/bitcoin/bitcoin/pull/14978#issuecomment-459373984 to add the new higher-method in Node interface, will add a commit, just need more thought to do it cleanly
ACKs for top commit:
MarcoFalke:
re-ACK fb62f128bbfd8c6cd72ea8e23331a4bae23883ab
Sjors:
re-ACK fb62f128bbfd8c6cd72ea8e23331a4bae23883ab
Tree-SHA512: a7ee48b0545f537fa65cac8ed4cb24e777ab90b877d4eefb87971fa93c6a59bd555b62ad8940c6ffb40592a0bd50787d27587af99f20b56af72b415b6394251f
|
|
d6b3640ac732f6f66a8cb6761084d1beecc8a876 [test] walletcreatefundedpsbt: check RBF is disabled when -walletrbf=0 (Sjors Provoost)
9ed062b5685eb6227d694572cb0f7bfbcc151b36 [doc] rpc: remove "fallback to" from RBF default help (Sjors Provoost)
4fcb698bc2bb74171cd3a14b94f9882d8e19e9fb [rpc] walletcreatefundedpsbt: use wallet default RBF (Sjors Provoost)
Pull request description:
The `walletcreatefundedpsbt` RPC call currently ignores `-walletrbf` and defaults to not use RBF. This PR fixes that.
This PR also replaces UniValue in `ConstructTransaction` with a `bool` in preparation of moving this helper method out of the RPC codebase entirely. This may be a bit overkill, but does slightly simplify it.
Fixes #15878
ACKs for top commit:
achow101:
Code Review ACK d6b3640ac732f6f66a8cb6761084d1beecc8a876
l2a5b1:
re-ACK d6b3640
MarcoFalke:
ACK d6b3640ac732f6f66a8cb6761084d1beecc8a876
Tree-SHA512: 55b9bccd1ef36b54f6b34793017dc0721103099ad3761b3b04862291ee13d6915915d4dbb1a8567924fa56e5e95dfe10eec070e06701610e70c87f8ea92b2a00
|
|
IsCoinBase check is already performed early by
AcceptToMemoryPoolWorker
GetDepthInMainChain check is already perfomed by
BroadcastTransaction
To avoid deadlock we MUST keep lock order in
ResendWalletTransactions and CommitTransaction,
even if we lock cs_main again further.
in BroadcastTransaction. Lock order will need
to be clean at once in a future refactoring
|
|
Higher wallet-tx method combining RelayWalletTransactions and
AcceptToMemoryPool, using new Chain::broadcastTransaction
|
|
7a0c2242890549b6dddac4cc6f1840a528469f2a Suppress output in test_bitcoin for expected errors (Gert-Jaap Glasbergen)
Pull request description:
Closes #15944
This adds two methods to noui, that allows temporarily suppressing (and then resuming) the output from `noui`. For situations where errors are expected, it's confusing for the test binary to output an error and then conclude with `No errors detected`.
It also uses this supress/reconnect in the tests that currently produce verbose errors when running `test_bitcoin`.
Output of `test_bitcoin` on current master:
```
gertjaap@gjdesktop:~/src/bitcoin$ src/test/test_bitcoin
Running 351 test cases...
Error: Specified -walletdir "/tmp/test_common_Bitcoin Core/1561389554_943311758/tempdir/path_does_not_exist" does not exist
Error: Specified -walletdir "/tmp/test_common_Bitcoin Core/1561389554_643733972/tempdir/not_a_directory.dat" is not a directory
Error: Specified -walletdir "wallets" is a relative path
*** No errors detected
```
Output after this code is merged:
```
gertjaap@gjdesktop:~/src/bitcoin$ src/test/test_bitcoin
Running 351 test cases...
*** No errors detected
```
ACKs for top commit:
l2a5b1:
ACK 7a0c224 - tested and reviewed.
laanwj:
ACK 7a0c2242890549b6dddac4cc6f1840a528469f2a
Tree-SHA512: c7881f7a431a065329360ffa9937ce4742694c646c90c019d3aff95dfd7fccbdcda9116c5762feb6dfd1108d14f9fb386e203b173c4bde9093afb2b8c977d13d
|
|
wallets
c5d37873677551caac34752214dd491f5278c8d5 Allow createwallet to take empty passwords to make unencrypted wallets (Andrew Chow)
Pull request description:
Allow createwallet to take the empty string as a password and interpret that as leaving the wallet unencrypted. Also warn when that happens.
This fixes a bug where it was not possible to use the `avoid_reuse` option for new unencrypted wallets without using named arguments.Thus this allows more `createwallet` options to be added that can be set on unencrypted wallets when using positional arguments.
ACKs for top commit:
jnewbery:
code review ACK c5d37873677551caac34752214dd491f5278c8d5
meshcollider:
re-utACK c5d37873677551caac34752214dd491f5278c8d5
ryanofsky:
utACK c5d37873677551caac34752214dd491f5278c8d5. Changes since last review are rebasing, concatenating warning strings to avoid discarding warnings, adding release notes, and choosing an unambiguous wallet name for the test.
Tree-SHA512: 146737a728dd614ba94d4b166b27e8c9e195badd1709ccab2315afe59176d9b493dfba9b61c3ed81090f059c7e464d709deb06d99451b9a3fff667f527d6f7c9
|
|
0b1f4b3c6685d0a6307926d43d166add538061b7 wallet: Drop unused OldKey (João Barbosa)
Pull request description:
Replaces #16494, `OldKey` (previously `CWalletKey`) was never serialized in the code history which means that unserialization support is not required, so remove the code entirely.
ACKs for top commit:
jnewbery:
ACK 0b1f4b3c6685d0a6307926d43d166add538061b7
laanwj:
ACK 0b1f4b3c6685d0a6307926d43d166add538061b7
fanquake:
ACK 0b1f4b3c6685d0a6307926d43d166add538061b7
Tree-SHA512: 92e9b2d6fc41f2765492d5d69d18fc4302c40ab44f28c8c30ca652c72767fbc484848c51a38ecf1f447849767a583c398784408bb5f64f9c86f9a5872b325ffc
|
|
|
|
80ba4241a6773590f6b2c18dae758097b5adc02e extract min & max depth onto coin control (Amiti Uttarwar)
Pull request description:
- Refactor `AvailableCoins` to pull min & max depths from coin control.
- Add `m_max_depth` to coin control to support this.
- Addresses issue https://github.com/bitcoin/bitcoin/issues/15823, see thread for further details.
ACKs for top commit:
laanwj:
ACK 80ba4241a6773590f6b2c18dae758097b5adc02e
Tree-SHA512: 8f7c0aa90b3bc3667baf6741b1da2829f3919e1df92ae097d86c6b239f0c024eb410d7100e6251ea8fc49d022fb5a1214bf79b0f8b0014945b7784b2311647d1
|
|
CMerkleTx is only used for deserialization of old wallet files. Remove
the serialization logic, and tidy up CWalletTx serialization logic.
|
|
Removes CMerkleTx as a base class for CWalletTx. Serialization logic is
moved from CMerkleTx to CWalletTx.
|
|
CMerkleTx only exists as a base class for CWalletTx and for wallet file
serialization/deserialization. Move CMerkleTx methods into CWalletTx,
but leave class hierarchy and serialization logic in place.
|
|
|
|
fa6f22bf44c0f741285f27f27ac18e9679802e5e wallet: Rename CWalletKey to OldKey (MarcoFalke)
fa6dc7fa5fdfd76c6dd5f7ae693a2fb4e37e271f wallet: Enumerate walletdb keys (MarcoFalke)
Pull request description:
It is nice to see all the keys that exists in a single enum
Also, rename CWalletKey to OldKey and update the outdated documentation
ACKs for top commit:
laanwj:
ACK fa6f22bf44c0f741285f27f27ac18e9679802e5e, I'm a big fan of this kind of change as it prevents typos, which can happen with 'magic' strings in the code.
promag:
ACK fa6f22bf44c0f741285f27f27ac18e9679802e5e. @jnewbery suggestions are great followups, I think this is good enough.
meshcollider:
utACK fa6f22bf44c0f741285f27f27ac18e9679802e5e
achow101:
Code review ACK fa6f22bf44c0f741285f27f27ac18e9679802e5e
fanquake:
ACK fa6f22bf44c0f741285f27f27ac18e9679802e5e - I had a quick look over, definitely prefer this to strings floating around everywhere.
Tree-SHA512: 8ac3abd5a0d22dac1d77b8f97fe1e16c2608d650f3e9d6dd1df2fd5aeb35ef6643dfd4cd5c162404bb0100343c927d66df04dc695507ffc84a6c667e603acc54
|
|
Allow createwallet to take the empty string as a password and interpret that
as leaving the wallet unencrypted. Also warn when that happens.
|
|
e967cae8fac84ec7a89a3a853a83d8193ac3308e Use switch on status in RpcWallet (Fabian Jahr)
ba1f128d6c117a63d5d904b3956551bd83405ec9 Return error for ignored passphrase through disable private keys option (Fabian Jahr)
d6649d16b57e20b05075f1c80d0de7ff32cca1a4 Use strong enum for WalletCreationStatus (Fabian Jahr)
3199610ad3b93b849f2cb55a8ed3a39a32bbdffc Place out args at the end for CreateWallet (Fabian Jahr)
Pull request description:
This is a follow-up PR to #16244
The following suggestions are included:
- Usage of `enum class` (https://github.com/bitcoin/bitcoin/pull/16244#discussion_r296434142)
- Placing out args at the end convention (https://github.com/bitcoin/bitcoin/pull/16244#discussion_r296434172)
- Return error when passphrase would be ignored because of disabled private keys (including functional test) (https://github.com/bitcoin/bitcoin/pull/16244#pullrequestreview-252015195)
- Make `status` return variable of `CreateWallet` (https://github.com/bitcoin/bitcoin/pull/16244#discussion_r302107394)
- Using a `switch` statement instead of `if/else` in `RpcWallet` (https://github.com/bitcoin/bitcoin/pull/16244#discussion_r302112502)
Not included was:
- "new create wallet function [could take] separate option arguments instead of wallet flags" (https://github.com/bitcoin/bitcoin/pull/16244#pullrequestreview-252015195)
- "blank wallet and disable private keys options could be combined into a single option" (https://github.com/bitcoin/bitcoin/pull/16244#pullrequestreview-252015195)
For these last two changes, I was not sure what an ideal solution could look like and/or this might be of slightly larger scope than the other changes, but I would be happy to work on these as well in this PR or another follow-up if I get positive feedback on that. Is there a place in the codebase that handles flags like these in a better way that I can refer to? Nonetheless, I would prefer keeping it in a separate PR unless it is a really simple change.
ACKs for top commit:
jnewbery:
Code review utACK e967cae8fac84ec7a89a3a853a83d8193ac3308e
MarcoFalke:
ACK e967cae8fa
Tree-SHA512: 3d12880ff95add9e4a5702afa26ef38080b57b216a608c113a4d0a08ba2d61142c027ba0071c6402add45db90383eee0bada12dc42820dc0d602721d7175edd5
|
|
|
|
|
|
|
|
|
|
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/unsigned int flags, const bool debug_only,/unsigned int flags,/' src/util/system.h src/util/system.cpp
sed -i 's/ArgsManager::NONE, debug_only/flags, false/' src/util/system.cpp
sed -i 's/arg.second.m_debug_only/(arg.second.m_flags \& ArgsManager::DEBUG_ONLY)/' src/util/system.cpp
sed -i 's/ArgsManager::ALLOW_ANY, true, OptionsCategory::/ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::/' $(git grep --files-with-matches 'AddArg(' src)
sed -i 's/ArgsManager::ALLOW_ANY, false, OptionsCategory::/ArgsManager::ALLOW_ANY, OptionsCategory::/' $(git grep --files-with-matches 'AddArg(' src)
-END VERIFY SCRIPT-
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/const bool debug_only,/unsigned int flags, &/' src/util/system.h src/util/system.cpp
sed -i -E 's/(true|false), OptionsCategory::/ArgsManager::ALLOW_ANY, &/' $(git grep --files-with-matches 'AddArg(' src)
-END VERIFY SCRIPT-
|
|
expose the "version" record
35e60e790f2cd602d1bdd0be835d27f0ba37efa9 Remove ReadVersion and WriteVersion (Andrew Chow)
b3d4f6c9619142948ab3d53551b4f3c0d7d73bde Log the actual wallet file version (Andrew Chow)
c88e87c3b2be3f97b712107e04285d06dfef3878 Remove nFileVersion from CWalletScanState (Andrew Chow)
Pull request description:
The wallet file version is stored in the "minversion" record, not the "version" record. However "version" is no longer used anywhere except to record the highest versioned client which has opened a wallet file (which is currently only used to check whether this was most recently opened by a 0.4.0 or 0.5.0rc1 client which had a broken wallet encryption implementation). Furthermore, "version" was logged to the debug.log which is confusing because it is not the actual wallet file version.
This PR changes it so that this confusion largely no longer exists. The wallet file version logging is changed to use "minversion" and reading and writing the "version" record is no longer publicly exposed to prevent potential confusion about whether the actual file version is being read or written. Lastly, in the one place it is actually used, the variable name is changed from nFileVersion to last_client to better reflect what that record actually represents.
ACKs for top commit:
jb55:
ACK 35e60e7, I compiled locally as a quick sanity check.
ryanofsky:
utACK 35e60e790f2cd602d1bdd0be835d27f0ba37efa9. This code still pretty confusing, but a little simpler now. And the previous log statement was really misleading and useless compared to the new one here.
meshcollider:
Looks good, thanks! utACK 35e60e790f2cd602d1bdd0be835d27f0ba37efa9
Tree-SHA512: f782b2f215d07fbc9b806322bda8085445b81c02b65ca674a8c6a3e1de505a0abd050669afe0ead4778816144a1c18462e13930071cedb7227a058aeb39493f7
|
|
fa4a605a4c611abe9af4c18aab20f4d1d039170f Remove wallet settings from chainparams (MarcoFalke)
Pull request description:
Feels a bit odd to have wallet setting in the chainparams, so remove them from there
ACKs for top commit:
promag:
ACK fa4a605a4c611abe9af4c18aab20f4d1d039170f, missed s/2018/2019?
practicalswift:
utACK fa4a605a4c611abe9af4c18aab20f4d1d039170f
darosior:
ACK fa4a605a4c611abe9af4c18aab20f4d1d039170f
Tree-SHA512: 2b3a5ee85d36af290d7db80bed1339e3c684607f1ce61cc65c906726e9174e40325fb1f67a34d8780f2a61fa39a1785e7c3a1cef5b6d6c364f38db5300cdbe3a
|
|
2f7eb772f6250442d4a0071318047cb2deeb31fa Add RPC bumpfee totalFee deprecation test (Jon Atack)
a92d9ce8cf355e18e43e1f207e4be9e42e7ec81a deprecate totalFee argument in bumpfee RPC call (Gregory Sanders)
Pull request description:
totalFee argument is of questionable use, and should be removed in favor of feerate-based features.
I first moved IsDeprecatedRPCEnabled because `bitcoin-wallet` doesn't link `libbitcoin_server`.
ACKs for top commit:
ryanofsky:
utACK 2f7eb772f6250442d4a0071318047cb2deeb31fa. Only change since last review is leaving IsDeprecatedRPCEnabled in its happy home, and switching to rpcEnableDeprecated instead. (Thanks!)
jonatack:
ACK 2f7eb772f6250442d4a0071318047cb2deeb31fa. Built locally, manually tested rpc bumpfee, help output ([gist](https://gist.github.com/jonatack/863673eacc02f9da39ff6d6712f9d837)), all tests pass. Travis failures appears to be unrelated, the [bitcoin builds are green](https://bitcoinbuilds.org/index.php?build=121).
meshcollider:
Code Review ACK 2f7eb772f6250442d4a0071318047cb2deeb31fa
Tree-SHA512: c97465205ee59575df37894bcbb6c4ecf8858dd8fe9d89503f9342b226768c1dcb553153bc9eb3055f7bf5eb41573e48b8efa57e083cd255793cbe5280f0026a
|
|
40ad2f6a58228c72c655e3061a19a63640419378 Have importwallet use ImportPrivKeys and ImportScripts (Andrew Chow)
78941da5baf6244c7c54e86cf8ce3e09ce60c239 Optionally allow ImportScripts to set script creation timestamp (Andrew Chow)
94bf156f391759420465b2ff8c44f5f150246c7f Have importaddress use ImportScripts and ImportScriptPubKeys (Andrew Chow)
a00d1e5ec5eb019f8bbeb060a2b09e341d360fe5 Have importpubkey use CWallet's ImportScriptPubKeys and ImportPubKeys functions (Andrew Chow)
c6a827424711333f6f66cf5f9d79e0e6884769de Have importprivkey use CWallet's ImportPrivKeys, ImportScripts, and ImportScriptPubKeys (Andrew Chow)
fae7a5befd0b8746d84a6fde575e5b4ea46cb3c4 Log when an import is being skipped because we already have it (Andrew Chow)
ab28e31c9563bd2cd1e4a088ffd2479517dc83f2 Change ImportScriptPubKeys' internal to apply_label (Andrew Chow)
Pull request description:
#15741 introduced `ImportPrivKeys`, `ImportPubKeys`, `ImportScripts`, and `ImportScriptPubKeys` in `CWallet` which are used by `importmulti`. This PR changes the remaining `import*` RPCs (`importaddress`, `importprivkey`, `importpubkey`, and `importwallet`) to use these functions as well instead of directly adding the imported items to the wallet.
ACKs for top commit:
MarcoFalke:
ACK 40ad2f6a58228c72c655e3061a19a63640419378 (checked that behavior changes are mentioned in the commit body)
ryanofsky:
utACK 40ad2f6a58228c72c655e3061a19a63640419378. Only change since last review is a tweaked commit message (mentioning label update in importpubkey commit)
Sjors:
ACK 40ad2f6a5. Those extra tests also pass.
Tree-SHA512: 910e3bbe20b6f8809a47b7293775db234125615d886c7fd99c194f4cdf00c765eb1e24b1799260f1213b98c88f9bbe696796f36087c182925e567d44e9194c98
|
|
|
|
Behavior changes:
* An "Importing ..." line is logged for every key, even ones that are skipped
|
|
Behavior changes:
* scripts imported in importmulti that are not explicilty scriptPubKeys will have timestamps set for them
|
|
Also removes the now unused ImportAddress and ImportScript from rpcdump.cpp
Behavior changes:
* No errors will be thrown when the script or key already exists in the wallet.
* If the key or script is already in the wallet, their labels will be updated.
|
|
Behavior changes:
* If any scripts for the pubkey were already in the wallet, their timestamps will be set to 1 and label updated
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/inline std::string _(const char\* psz)/inline bilingual_str _(const char\* psz)/' src/util/translation.h
sed -i 's/return G_TRANSLATION_FUN ? (G_TRANSLATION_FUN)(psz) : psz;/return bilingual_str{psz, G_TRANSLATION_FUN ? (G_TRANSLATION_FUN)(psz) : psz};/' src/util/translation.h
sed -i 's/\b_("\([^"]\|\\"\)*")/&.translated/g' $(git grep --files-with-matches '\b_("' src)
echo Hard cases - multiline strings.
sed -i 's/"Visit %s for further information about the software.")/&.translated/g' src/init.cpp
sed -i "s/\"Only rebuild the block database if you are sure that your computer's date and time are correct\")/&.translated/g" src/init.cpp
sed -i 's/" restore from a backup.")/&.translated/g' src/wallet/db.cpp
sed -i 's/" or address book entries might be missing or incorrect.")/&.translated/g' src/wallet/wallet.cpp
echo Special case.
sed -i 's/_(COPYRIGHT_HOLDERS)/&.translated/' src/util/system.cpp test/lint/lint-format-strings.py
-END VERIFY SCRIPT-
|
|
This is a prerequisite for introducing bilingual error messages.
Note: #includes are arranged by clang-format-diff.py script.
|
|
|
|
The "version" record that these functions read and write are not
used anywhere in the code except for one place. There is no reason
to expose these functions publicly. Furthermore, this avoids potential
confusion as developers may mistake these functions for actually
reading and writing the wallet version when they do not.
|
|
The actual wallet file version is the minversion record, not the
version record.
|
|
nFileVersion is not the actual file version and is not used except
in one place. So it is removed from CWalletScanState and changed so
that it is just read at the place it is needed. Furthermore, the
"version" record now only indicates the version of the highest
versioned client that has opened a wallet file so the variable
name is changed accordingly
|
|
|
|
|
|
ImportScriptPubKeys
Behavior changes:
* If we already have the key, it's wpkh script will still be added, although it should already be there
|
|
Behavior Changes:
* Those pubkeys being imported with add_keypool set and are already in the wallet will no longer be added to the keypool
|
|
e10e1e8db043e9b7c113e07faf408f337c1b732d Restrict lifetime of ReserveDestination to CWallet::CreateTransaction (Gregory Sanders)
d9ff862f2d24784ee081a8f62a76ffdfe409c10a CreateTransaction calls KeepDestination on ReserveDestination before success (Gregory Sanders)
Pull request description:
The typical usage pattern of `ReserveDestination` is to explicitly `KeepDestination`, or `ReturnDestination` when it's detected it will not be used.
Implementers such as myself may fail to complete this pattern, and could result in key re-use: https://github.com/bitcoin/bitcoin/pull/15557#discussion_r271956393
Since ReserveDestination is currently only used directly in the `CreateTransaction`/`CommitTransaction` flow(or fee bumping where it's just used in `CreateTransaction`), I instead make the assumption that if a transaction is returned by `CreateTransaction` it's highly likely that it will be accepted by the caller, and the `ReserveDestination` kept. This simplifies the API as well. There are very few cases where this would not be the case which may result in keys being burned.
Those failure cases appear to be:
`CommitTransaction` failing to get the transaction into the mempool
Belt and suspenders check in `WalletModel::prepareTransaction`
Alternative to https://github.com/bitcoin/bitcoin/pull/15796
ACKs for top commit:
achow101:
ACK e10e1e8db043e9b7c113e07faf408f337c1b732d Reviewed the diff
stevenroose:
utACK e10e1e8db043e9b7c113e07faf408f337c1b732d
meshcollider:
utACK e10e1e8db043e9b7c113e07faf408f337c1b732d
Tree-SHA512: 78d047a00f39ab41cfa297052cc1e9c224d5f47d3d2299face650d71827635de077ac33fb4ab9f7dc6fc5a27f4a68415a1bc9ca33a3cb09a78f4f15b2a48411b
|
|
|
|
|