| Age | Commit message (Collapse) | Author |
|---|
|
c812aba3949b6ab81030dc708cda7c8821be2f70 test bumpfee fee_rate argument (ezegom)
9f25de3d9eb8d012ca1a98cbcd28021e3e1c85ee rpc bumpfee check fee_rate argument (ezegom)
88e5f997dfab3f03bb1ec3f149eaff8dcc2981fe rpc bumpfee: add fee_rate argument (ezegom)
1a4c791cf49ff15aa9deba4388c0180b8f47f15b rpc bumpfee: move feerate estimation logic into separate method (ezegom)
Pull request description:
Taking over for https://github.com/bitcoin/bitcoin/pull/16492 which seems to have gone inactive.
Only minor commit cleanups, rebase, and some help text fixes on top of previous PR. Renamed `feeRate` to `fee_rate` to reflect updated guidelines.
ACKs for top commit:
Sjors:
Code review ACK c812aba
laanwj:
ACK c812aba3949b6ab81030dc708cda7c8821be2f70
Tree-SHA512: 5f7f51bd780a573ccef1ccd72b0faf3e5d143f6551060a667560c5163f7d9480e17e73775d1d7bcac0463f3b6b4328f0cff7b27e39483bddc42a530f4583ce30
|
|
80031045fc6435ef4eb5dd1aee773d938c57a0fd Clarify includeWatching for fundrawtransaction (Steven Roose)
Pull request description:
Might be sufficient to solve https://github.com/bitcoin/bitcoin/issues/16396, https://github.com/bitcoin/bitcoin/issues/7879 and https://github.com/bitcoin/bitcoin/issues/14405.
ACKs for top commit:
Sjors:
ACK 8003104. This will always be confusing, but at least it gives a bunch more clues for the user to google.
Tree-SHA512: 9b8002c259c50f93d89fc5574105aae6152858d8d45c07b4c3d5b7023adafe73c7a98a290874ff3fbbb7dfad2ac1bdf4acb8769a2a1c14e38484922f44e84e54
|
|
|
|
PR 16866 renamed the 'decode' argument in gettransaction to 'verbose' to make it more consistent with other RPC calls like getrawtransaction.
However, it seems it inadvertently overloaded the 'details' fields when 'verbose' is passed. The result is that the original 'details' fields are no longer returned, which seems to be a breaking API change.
This PR takes the simplest path to restoring the 'details' fields by renaming them from 'details' back to 'decoded', while leaving the 'verbose' argument for API consistency.
It also addresses [this comment](https://github.com/bitcoin/bitcoin/pull/16185#discussion_r320740413) to mention that the 'decoded' field is identical to decoderawtransaction.
Update the RPC help, functional test, and release note.
|
|
This makes the RPC method consistent with other RPC methods that have a
'verbose' option.
Change the name of the return object from 'decoded' to details.
Update help text.
|
|
|
|
prevtx parsing
39034f1ee628dae0bc9da5b1b30b8a424e66d968 Refactor rawtransaction_util's SignTransaction to have previous tx parsing be separate (Andrew Chow)
Pull request description:
Currently the `SignTransaction` function has to handle both the actual signing and parsing of previous transaction data. This PR splits it so that `SignTransaction` only handles the signing itself and adds a `ParsePrevouts` function which handles parsing the prevtx information.
This allows for `SignTransaction` to just take any `SigningProvider`.
Split from #16341
ACKs for top commit:
MarcoFalke:
ACK 39034f1ee628dae0bc9da5b1b30b8a424e66d968
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/16798/commits/39034f1ee628dae0bc9da5b1b30b8a424e66d968
ryanofsky:
utACK 39034f1ee628dae0bc9da5b1b30b8a424e66d968. No change since previously reviewed b49bbb939be92a67ff77c3f7bca5bb94dd141906, https://github.com/bitcoin/bitcoin/pull/16341#pullrequestreview-278610269 other than rebase with no conflicts.
Tree-SHA512: 09f7733e90691766bfb5cf0f20e913dbf270bd3b51abdcad966b24d110e562ed85fd3d0d1d7bbea61f903340060052ec73c4817b09aee0dc1f3916d781a9e40c
|
|
442a87cc0ae43ebc9b6654a6165778eecb931f74 Add a test wallet_reorgsrestore (Antoine Riard)
40ede992d97df38282919693dfe851c975c3b1d8 Modify wallet tx status if has been reorged out (Antoine Riard)
7e89994133725125dddbfa8d45484e3b9ed51c6e Remove SyncTransaction for conflicted txn in CWallet::BlockConnected (Antoine Riard)
a31be09bfd77eed497a8e251d31358e16e2f2eb1 Encapsulate tx status in a Confirmation struct (Antoine Riard)
Pull request description:
While working on #15931, I've tried to rationalize tx state management to ease integration of block height tracking per-wallet tx. We currently rely on a combination of `hashBlock` and `nIndex` with magic value to determine tx confirmation, conflicted or abandoned state. It's hard to reason and error-prone. To solve that, we encapsulate these fields in a `TxConfirmation` struct and introduce a `TxState` member that we update accordingly at block connection/disconnection.
Following jnewbery [recommendation](https://github.com/bitcoin/bitcoin/pull/15931#discussion_r312576506), I've taken these changes in its own commit, and open a PR to get them first. It would ease review of aforementioned PR, but above all should ease fixing of long-term issues like :
* https://github.com/bitcoin/bitcoin/issues/7315 (but maybe we should abandon abandontransaction or relieve it to only free outpoints not track the transaction as abandoned in itself, need its own discussion)
* https://github.com/bitcoin/bitcoin/issues/8692 where we should cancel conflicted state of transactions chain smoothly
* `MarkConflicted` in `LoadToWallet` is likely useless if we track conflicts rights at block connection
Main changes of this PR to get right are tx update in `AddToWallet` and serialization/deserialization logic.
ACKs for top commit:
meshcollider:
Light re-Code Review ACK 442a87cc0ae43ebc9b6654a6165778eecb931f74
ryanofsky:
utACK 442a87cc0ae43ebc9b6654a6165778eecb931f74. Changes since last review are switching from `hasChain` to `LockChain` and removing chain lock in `WalletBatch::LoadWallet` that's redundant with the new lock still added in `CWallet::LoadWallet`, and fixing python test race condition.
Tree-SHA512: 029209e006de0240436817204e69e548c5665e2b0721b214510e7aba7eba130a5eab441d3a1ad95bd6426114dd27390492c77bf4560a9610009b32cd0a1f72f7
|
|
be separate
|
|
This adds a new boolean parameter 'decode' to the gettransaction call, which, if set to true, add a 'decoded' field to the result containing the decoded transaction
|
|
Instead of relying on combination of hashBlock and nIndex
values to manage tx in its lifecycle, we introduce 4
status : CONFIRMED, UNCONFIRMED, CONFLICTED, ABANDONED.
hashBlock and nIndex magic values should only be used at
serialization/deserialization for backward-compatibility.
At block disconnection, we know flag txn as UNCONFIRMED where
previously they kept their states until being override by a
block connection or abandontransaction call. This is a change
in behavior for which user may have to call abandon twice
if transaction is disconnected and not accepted back in the mempool.
We assert status transitioning right in AddToWallet. Doing so
flagged a misbehavior in ComputeTimeSmart unit test where same
tx is confirmed twice in different block. To avoid inconsistencies
we unconfirmed tx before new connection in different block. We
also remove a cs_main lock in test, as AddToWallet and its
callees don't rely on locked chain.
|
|
72eaab073bc747425fe551777154b13a6c4c37c9 tests: functional watch-only wallet tests (William Casarin)
72ffbdc5799c1707ecad674d701b43fb80b031d0 doc: add release note for include_watchonly default changes (William Casarin)
003a3c73c0450aa18ac2ab2ca47def2b8c53a7df rpcwallet: document include_watchonly default for watchonly wallets (William Casarin)
a50d9e6c0b8e8144d3deec58ec2e3449ba081151 rpcwallet: default include_watchonly to true for watchonly wallets (William Casarin)
Pull request description:
Right now it's a bit annoying to deal with watchonly wallets, many rpc commands have an `include_watchonly` argument that needs to be explicitly set.
Wallets created with `createwallet` can have a `disable_private_keys` parameter, for those wallets we already know that they are watchonly, so there's no reason to have to explicitly ask for it for every command. Instead we check this wallet flag when the `include_watchonly` parameter isn't set.
ACKs for top commit:
achow101:
Code review ACK 72eaab073bc747425fe551777154b13a6c4c37c9
Sjors:
ACK 72eaab073bc747425fe551777154b13a6c4c37c9
promag:
ACK 72eaab073bc747425fe551777154b13a6c4c37c9, code review only, didn't look closely to the test.
kallewoof:
ACK 72eaab073bc747425fe551777154b13a6c4c37c9
fanquake:
ACK 72eaab073bc747425fe551777154b13a6c4c37c9 - I've looked over the changes, they make sense to me. Compiled and ran the tests etc.
Tree-SHA512: d3646b55e97f386594d7efc994f0712f3888475c6a5dc7f131ac9f8c49bf5d4677182b88f42b34152abe1ad101ecadd152b4c20e9d3c1267190db36f77ab8bd7
|
|
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.
|
|
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
|
|
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
|
|
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
|
|
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
|
|
|
|
|
|
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
|
|
|
|
|
|
|
|
|
|
Signed-off-by: William Casarin <jb55@jb55.com>
|
|
The logic before would only include watchonly addresses if it was
explicitly set in the rpc argument.
This changes the logic like so:
If the include_watchonly argument is missing, check the
WALLET_FLAG_DISABLE_PRIVATE_KEYS flag to determine if we're working
with a watchonly wallet. If so, default include_watchonly to true.
If the include_watchonly argument is explicit set to false, we still
disable them from the listing. Although this would always return
nothing, it might be still useful in situations where you want to
explicitly filter out watchonly addresses regardless of what wallet
you are dealing with.
Signed-off-by: William Casarin <jb55@jb55.com>
|
|
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
|
|
|
|
93ce4a0b6fb54efb1f424a71dfc09cc33307e5b9 Move WatchOnly stuff from SigningProvider to CWallet (Andrew Chow)
8f5b81e6edae9cb22559545de63f391d97c15701 Remove CCryptoKeyStore and move all of it's functionality into CWallet (Andrew Chow)
37a79a4fccbf6cd65a933594e24e59d36e674653 Move various SigningProviders to signingprovider.{cpp,h} (Andrew Chow)
16f8096e911e4d59292240a17e2d4004f0500b9e Move KeyOriginInfo to its own header file (Andrew Chow)
d9becff4e13da8e182631baa79b9794c03d44434 scripted-diff: rename CBasicKeyStore to FillableSigningProvider (Andrew Chow)
a913e3f2fbeb1352fc66f334d4f5f7332ea89ad7 Move HaveKey static function from keystore to rpcwallet where it is used (Andrew Chow)
c7797ec65544bd23a2e571b2892e1bf512f2a485 Remove CKeyStore and squash into CBasicKeyStore (Andrew Chow)
1b699a5083b435c2b79f3951f94ac9f967d24f6c Add HaveKey and HaveCScript to SigningProvider (Andrew Chow)
Pull request description:
This PR compresses the `CWallet` chain of inheritance from 5 classes to 3 classes. `CBasicKeyStore` is renamed to `FillableSigningProvider` and some parts of it (the watchonly parts) are moved into `CWallet`. `CKeyStore` and `CCrypoKeyStore` are completely removed. `CKeyStore`'s `Have*` functions are moved into `SigningProvider` and the `Add*` moved into `FillableSigningProvider`, thus allowing it to go away entirely. `CCryptoKeyStore`'s functionality is moved into `CWallet`. The new inheritance chain is:
```
SigningProvider -> FillableSigningProvider -> CWallet
```
`SigningProvider` now is the class the provides keys and scripts and indicates whether keys and scripts are present. `FillableSigningProvider` allows keys and scripts to be added to the signing provider via `Add*` functions. `CWallet` handles all of the watchonly stuff (`AddWatchOnly`, `HaveWatchOnly`, `RemoveWatchOnly` which were previously in `CKeyStore`) and key encryption (previously in `CCryptoKeyStore`).
Implements the 2nd [prerequisite](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes#cwallet-subclass-stack) from the wallet restructure.
ACKs for top commit:
Sjors:
re-ACK 93ce4a0; it keeps `EncryptSecret`, `DecryptSecret` and `DecryptKey` in `wallet/crypter.cpp`, but makes them not static. It improves alphabetical includes, reorders some function definitions, fixes commit message, brings back lost code comment.
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/16227/commits/93ce4a0b6fb54efb1f424a71dfc09cc33307e5b9
Tree-SHA512: 393dfd0623ad2dac38395eb89b862424318d6072f0b7083c92a0d207fd032c48b284f5f2cb13bc492f34557de350c5fee925da02e47daf011c5c6930a721b6d3
|
|
function
1aecdf2063cbe28d4715ae5ae1a7e51b860c9f4d Move wallet creation out of the createwallet rpc into its own function (Andrew Chow)
Pull request description:
Moves the wallet creation logic from within the `createwallet` rpc and into its own function within wallet.cpp.
ACKs for top commit:
jnewbery:
ACK 1aecdf2063cbe28d4715ae5ae1a7e51b860c9f4d
MarcoFalke:
ACK 1aecdf2063cbe28d4715ae5ae1a7e51b860c9f4d
Sjors:
ACK 1aecdf2 with some suggestions for followup.
Tree-SHA512: 8d26d7ff48db4f8fac12408a5a294f788b7f50a72e7eb4008fb74ff14d7400eb3970f8038a19f989eff55198fc11c0cf86f52231c62b9015eb777132edc8ea88
|
|
|
|
8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf Add GetNewChangeDestination for getting new change Destinations (Andrew Chow)
33d13edd2bda0af90660e275ea4fa96ca9896f2a Replace CReserveKey with ReserveDestinatoin (Andrew Chow)
172213be5b174243dc501c1103ad5fe2fee67a16 Add GetNewDestination to CWallet to fetch new destinations (Andrew Chow)
Pull request description:
The wallet should give out destinations instead of keys. It should be the one that handles the conversion from key to destination and the setting of the label, not the caller. In order to do this, two new member functions are introduced `GetNewDestination()` and `GetNewChangeDestination()`. Additionally, `CReserveKey` is changed to be `ReserveDestination` and represents destinations whose keys can be returned to the keypool.
ACKs for top commit:
instagibbs:
re-utACK https://github.com/bitcoin/bitcoin/pull/16237/commits/8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf
sipa:
ACK 8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf. Concept ACK as this gives a much cleaner abstraction to work with, and light code review ACK.
laanwj:
ACK 8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf
Tree-SHA512: 5be7051409232b71e0ef2c1fd1a3e76964ed2f5b14d47d06edc2ad3b3687abd0be2803a1adc45c0433aa2c3bed172e14f8a7e9f4a23bff70f86260b5a0497500
|
|
|
|
Adds a GetNewChangeDestination that has the same objective as GetNewDestination
|
|
Instead of reserving keys, reserve destinations which are backed by keys
|
|
Instead of having the same multiple lines of code everywhere
that new destinations are fetched, introduce GetNewDestination as
a member function of CWallet which does the key fetching, label
setting, script generation, and destination generation.
|
|
|
|
|
|
71d0344cf25d3aaf60112c5248198c444bc98105 docs: release note wording (Karl-Johan Alm)
3d2ff379131a01e4e9f9648b150e806104a23795 wallet/rpc: use static help text (Karl-Johan Alm)
53c3c1ea9e20f881c843a9219e48cec202e962f8 wallet/rpc/getbalances: add entry for 'mine.used' balance in results (Karl-Johan Alm)
Pull request description:
This addresses a few remaining issues pointed out in #13756:
* First commit addresses https://github.com/bitcoin/bitcoin/pull/13756#discussion_r284907468
* Second commit addresses https://github.com/bitcoin/bitcoin/pull/13756#discussion_r294868973
Ping jnewbery and achow101 as they pointed out these issues.
ACKs for commit 71d034:
jnewbery:
ACK 71d0344cf25d3aaf60112c5248198c444bc98105
meshcollider:
re-utACK https://github.com/bitcoin/bitcoin/pull/16239/commits/71d0344cf25d3aaf60112c5248198c444bc98105
Tree-SHA512: 5e28822af0574ad07dbbed21aa2fe7866bf5770b4c0a1c150ad0da8af3152bcfb7170330a7497fa500326c594740ecf63733cf58325821e2811d7b911d5783a0
|
|
Always show the same help topic regardless of wallet flags, and explain that something is not always available, rather than runtime-modifying the help output.
|
|
|
|
returns a legacy address
a49503402b6bc21e3878e151c07529941d36aed0 Make and get the multisig redeemscript and destination in one function instead of two (Andrew Chow)
Pull request description:
`CreateMultisigRedeemscript()` is changed to `AddAndGetMultisigDestination()` so that the process of constructing the redeemScript and then getting the `CTxDestination` are done in the same function. This allows that function to see what the keys in the multisig are so that the correct address type is returned from `AddAndGetDestinationForScript()`.
This only effects the `createmultisig` and `addmultisigaddress` RPCs and does not change signing logic as #16022 does.
Alternative to #16022 and #16012
Fixes #16011
ACKs for commit a49503:
Tree-SHA512: 5b0154a714deea3b2cc3a54beb420c95eeeacf4ca30c40ca80940d9d640f8b03611b0fc14c2f0710bfd8a79e8d27ad7d9ae380b4b83d52b40ab201624f2a63f0
|
|
instead of two
Instead of creating a redeemScript with CreateMultisigRedeemscript and
then getting the destination with AddAndGetDestinationForScript, do
both in the same function.
CreateMultisigRedeemscript is changed to AddAndGetMultisigDestination.
It creates the redeemScript and returns it via an output parameter. Then
it calls AddAndGetDestinationForScript to add the destination to the
keystore and get the proper destination.
This allows us to inspect the public keys in the redeemScript before creating
the destination so that the correct destination is used when uncompressed
pubkeys are in the multisig.
|
|
5ebc6b0eb267e0552c66fffc5e5afe7df8becf80 bitcoind: update -avoidpartialspends description to account for auto-enable for avoid_reuse wallets (Karl-Johan Alm)
ada258f8c8f92d44d893cf9f22d15acdeca40b1a doc: release notes for avoid_reuse (Karl-Johan Alm)
27669551da52099e4a6a401acd7aa32b32832423 wallet: enable avoid_partial_spends by default if avoid_reuse is set (Karl-Johan Alm)
8f2e208f7c0468f9ba92bc789a698281b1c81284 test: add test for avoidreuse feature (Karl-Johan Alm)
0bdfbd34cf4015de87741ff549db35e5064f4e16 wallet/rpc: add 'avoid_reuse' option to RPC commands (Karl-Johan Alm)
f904723e0d5883309cb0dd14b826bc45c5e776fb wallet/rpc: add setwalletflag RPC and MUTABLE_WALLET_FLAGS (Karl-Johan Alm)
8247a0da3a46d7c38943ee0304343ab7465305bd wallet: enable avoid_reuse feature (Karl-Johan Alm)
eec15662fad917b169f5e3b8baaf4301dcf00a7b wallet: avoid reuse flags (Karl-Johan Alm)
58928098c299efdc7c5ddf2dc20716ca5272f21b wallet: make IsWalletFlagSet() const (Karl-Johan Alm)
129a5bafd9a3efa2fa16d780885048a06566d262 wallet: rename g_known_wallet_flags constant to KNOWN_WALLET_FLAGS (Karl-Johan Alm)
Pull request description:
Add a new wallet flag called `avoid_reuse` which, when enabled, will keep track of when a specific destination has been spent from, and will actively "blacklist" any new UTXOs which send to an already-spent-from destination.
This improves privacy, as a payer could otherwise begin tracking a payee's wallet by regularly peppering a known UTXO with dust outputs, which would then be scooped up and used in payments by the payee, allowing the payer to map out (1) the inputs owned by the payee and (2) the destinations to which the payee is making payments.
This replaces #10386 and together with the (now merged) #12257 it addresses #10065 in full. The concerns raised in https://github.com/bitcoin/bitcoin/pull/10386#issuecomment-302361381 are also addressed due to #12257.
~~Note: this builds on top of #15780.~~ (merged)
ACKs for commit 5ebc6b:
jnewbery:
ACK 5ebc6b0eb
laanwj:
Concept and code-review ACK 5ebc6b0eb267e0552c66fffc5e5afe7df8becf80
meshcollider:
Code review ACK https://github.com/bitcoin/bitcoin/pull/13756/commits/5ebc6b0eb267e0552c66fffc5e5afe7df8becf80
achow101:
ACK 5ebc6b0eb267e0552c66fffc5e5afe7df8becf80 modulo above nits
Tree-SHA512: fdef45826af544cbbb45634ac367852cc467ec87081d86d08b53ca849e588617e9a0a255b7e7bb28692d15332de58d6c3d274ac003355220e4213d7d9070742e
|
|
fa499b5f027f77c0bf13699852c8c06f78e27bef rpc: bugfix: Properly use iswitness in converttopsbt (MarcoFalke)
fa5c5cd141f0265a5693234690ac757b811157d8 rpc: Switch touched RPCs to IsValidNumArgs (MarcoFalke)
Pull request description:
When a serialized transaction has inputs, there is no risk in only trying to deserialize it with witness allowed. (This is how all transactions from p2p are deserialized.) In fact, it would avoid a common issue where a transaction with inputs can be deserialized in two ways:
* Fixes #12989
* Fixes #15872
* Fixes #15701
* Fixes #13738
* ...
When a serialized transaction has no inputs, there is no risk in only trying to deserialze it with witness disallowed. (A transaction without inputs can't have corresponding witness data)
ACKs for commit fa499b:
meshcollider:
utACK https://github.com/bitcoin/bitcoin/pull/15899/commits/fa499b5f027f77c0bf13699852c8c06f78e27bef
ryanofsky:
utACK fa499b5f027f77c0bf13699852c8c06f78e27bef. Changes since last review: consolidating commits and making iswitness documentation the same across methods.
PastaPastaPasta:
utACK fa499b5f027f77c0bf13699852c8c06f78e27bef
Tree-SHA512: a64423a3131f3f0222a40da557c8b590c9ff01b45bcd40796f77a1a64ae74c6680a6be9d01ece95c492dfbcc7e2810409d2c2b336c2894af00bb213972fc85c6
|
|
67f4e9c522 Include core_io.h from core_read.cpp (practicalswift)
eca9767673 Make reasoning about dependencies easier by not including unused dependencies (practicalswift)
Pull request description:
Make reasoning about dependencies easier by not including unused dependencies.
Please note that the removed headers are _not_ "transitively included" by other still included headers. Thus the removals are real.
As an added bonus this change means less work for the preprocessor/compiler. At least 51 393 lines of code no longer needs to be processed:
```
$ git diff -u HEAD~1 | grep -E '^\-#include ' | cut -f2 -d"<" | cut -f1 -d">" | \
sed 's%^%src/%g' | xargs cat | wc -l
51393
```
Note that 51 393 is the lower bound: the real number is likely much higher when taking into account transitively included headers :-)
ACKs for commit 67f4e9:
Tree-SHA512: 0c8868aac59813f099ce53d5307eed7962dd6f2ff3546768ef9e5c4508b87f8210f1a22c7e826c3c06bebbf28bdbfcf1628ed354c2d0fdb9a31a42cefb8fdf13
|
|
|
|
|
|
|
