Age | Commit message (Collapse) | Author |
|
from transifex translator feedback
48742693acc9de837735674057c9aae2fe90bd1d Replace "can not" with "cannot" in docs, user messages, and tests (Jon Atack)
e670edd43441ecb6e5978d65348501c57d856030 User-facing content fixups from transifex translator feedback (Jon Atack)
Pull request description:
Closes #24366.
ACKs for top commit:
laanwj:
Code review re-ACK 48742693acc9de837735674057c9aae2fe90bd1d
hebasto:
re-ACK 48742693acc9de837735674057c9aae2fe90bd1d, only suggested change since my previous [review](https://github.com/bitcoin/bitcoin/pull/24367#pullrequestreview-885938219).
Tree-SHA512: 4dcdcb417251a413e65fab6070515e13a1267c8e0dbcf521386b842511391f24c84a0c2168fe13458c977682034466509bf2a3453719d4d94d3c568fd9f4adb4
|
|
|
|
|
|
|
|
|
|
When private keys are disabled, we should not be trying to generate new
keys during upgradewallet.
|
|
|
|
|
|
|
|
|
|
|
|
This should also fix an init error if a -walletdir with a trailing slash
is used on windows. This appears to be a real error and regression
introduced with #20744.
On windows (or at least wine), fs calls that actuallly access the
filesystem like fs::equivalent or fs::exists seem to treat directory
paths with trailing slashes as not existing, so it's necessary to
normalize these paths before using them. This change passes canonical
paths to fs calls validating the -walletdir path to fix this.
|
|
Warning: Replacing fs::system_complete calls with fs::absolute calls
in this commit may cause minor changes in behaviour because fs::absolute
no longer strips trailing slashes; however these changes are believed to
be safe.
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
|
|
|
|
3ee6d0788ec1b90f7c39c9644dba4011f7cf5db4 test: add more wallet conflicts assertions (S3RK)
3b98bf9c43ece060d57d7ae31624d4a8220de266 Revert "Add to spends only transcations from me" (S3RK)
Pull request description:
This reverts commit d04566415e16ae685af066384f346dff522c068f from #22929.
This commit was based on invalid assumption that `mapTxSpends` should contain only outgoing txs and broke wallet conflicts feature.
ACKs for top commit:
achow101:
ACK 3ee6d0788ec1b90f7c39c9644dba4011f7cf5db4
Tree-SHA512: bf5a77ced6bac57d5eb85771d9189c53e1edc295d179ed5a1bdce18e365794a9101b4cecf35387b27f67260db3b47f7214e7876e490494529b748cceeb95632d
|
|
|
|
|
|
rework BIP 65/68/112 docs
fa4339e4c1bb60e0d9263d4f0fe65d03aad52f88 Extract CTxIn::MAX_SEQUENCE_NONFINAL constant (MarcoFalke)
Pull request description:
Extracting the constant makes it possible to attach documentation to it.
Also, rework the docs for the other "sequence constants".
ACKs for top commit:
w0xlt:
reACK fa4339e for specifying the transaction version.
darosior:
re-ACK fa4339e4c1bb60e0d9263d4f0fe65d03aad52f88
luke-jr:
crACK fa4339e4c1bb60e0d9263d4f0fe65d03aad52f88
Tree-SHA512: 8d8f3dd5afb33eb5b72aa558e1e03de874c5ed02aa1084888e92ed86f3aaa5c725db45ded02e14cdfa67a92ac6774e97185b697f20a8ab63abbfcaa2fcd1fc6a
|
|
fa5d2e678c809c26bd40d7e7c171529d3ffb5903 Remove unused char serialize (MarcoFalke)
fa24493d6394b3a477535f480664c9596f18e3c5 Use spans of std::byte in serialize (MarcoFalke)
fa65bbf217b725ada35107b4ad646d250228355c span: Add BytePtr helper (MarcoFalke)
Pull request description:
This changes the serialize code (`.read()` and `.write()` functions) to take a `Span` instead of a pointer and size. This is a breaking change for the serialize interface, so at no additional cost we can also switch to `std::byte` (instead of using `char`).
The benefits of using `Span`:
* Less verbose and less fragile code when passing an already existing `Span`(-like) object to or from serialization
The benefits of using `std::byte`:
* `std::byte` can't accidentally be mistaken for an integer
The goal here is to only change serialize to use spans of `std::byte`. If needed, `AsBytes`, `MakeUCharSpan`, ... can be used (temporarily) to pass spans of the right type.
Other changes that are included here:
* [#22167](https://github.com/bitcoin/bitcoin/pull/22167) (refactor: Remove char serialize by MarcoFalke)
* [#21906](https://github.com/bitcoin/bitcoin/pull/21906) (Preserve const in cast on CTransactionSignatureSerializer by promag)
ACKs for top commit:
laanwj:
Concept and code review ACK fa5d2e678c809c26bd40d7e7c171529d3ffb5903
sipa:
re-utACK fa5d2e678c809c26bd40d7e7c171529d3ffb5903
Tree-SHA512: 08ee9eced5fb777cedae593b11e33660bed9a3e1711a7451a87b835089a96c99ce0632918bb4666a4e859c4d020f88fb50f2dd734216b0c3d1a9a704967ece6f
|
|
nStatus/nFile/nDataPos/nUndoPos by cs_main
6ea56827842b9b2bd730edc38f3a7b1f46f6247b Guard CBlockIndex::nStatus/nFile/nDataPos/nUndoPos by cs_main (Jon Atack)
5d59ae0ba88849b1eb0d7350871bc19fcd5ef601 Remove/inline ReadRawBlockFromDisk(block_data, pindex, message_start) (Hennadii Stepanov)
eaeeb88768db529b5241ccd42f1e87579908b4df Require IsBlockPruned() to hold mutex cs_main (Jon Atack)
ca47b005770f71aa229ecc1f7b8146a96ff02151 Require CBlockIndex::IsValid() to hold cs_main (Vasil Dimov)
e9f3aa5f6a7b39e8d5f2069617e5e382798d8d60 Require CBlockIndex::RaiseValidity() to hold cs_main (Vasil Dimov)
8ef457cb83fac796f8b6a56977b1016193fc1185 Require CBlockIndex::IsAssumedValid() to hold cs_main (Vasil Dimov)
572393448b4d32f91b92edc84b4200ab52d62422 Require CBlockIndex::GetUndoPos() to hold mutex cs_main (Jon Atack)
2e557ced2830fc54476e598d52225f1679205e7d Require WriteUndoDataForBlock() to hold mutex cs_main (Jon Atack)
6fd4341c10b319399c58d71c4ddeae4417e337d7 Require CBlockIndex::GetBlockPos() to hold mutex cs_main (Jon Atack)
Pull request description:
Issues:
- `CBlockIndex` member functions `GetBlockPos()`, `GetUndoPos()`, `IsAssumedValid()`, `RaiseValidity()`, and `IsValid()` and block storage functions `WriteUndoDataForBlock()` and `IsBlockPruned()` are missing thread safety lock annotations to help ensure that they are called with mutex cs_main to avoid bugs like #22895. Doing this also enables the next step:
- `CBlockIndex::nStatus` may be racy, i.e. potentially accessed by multiple threads, see #17161. A solution is to guard it by cs_main, along with fellow data members `nFile`, `nDataPos` and `nUndoPos`.
This pull:
- adds thread safety lock annotations for the functions listed above
- guards `CBlockIndex::nStatus`, `nFile`, `nDataPos` and `nUndoPos` by cs_main
How to review and test:
- debug build with clang and verify there are no `-Wthread-safety-analysis` warnings
- review the code to verify each annotation or lock is necessary and sensible, or if any are missing
- look for whether taking a lock can be replaced by a lock annotation instead
- for more information about Clang thread safety analysis, see
- https://clang.llvm.org/docs/ThreadSafetyAnalysis.html
- https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#lockingmutex-usage-notes
- https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#threads-and-synchronization
Mitigates/potentially closes #17161.
ACKs for top commit:
laanwj:
Code review ACK 6ea56827842b9b2bd730edc38f3a7b1f46f6247b
Tree-SHA512: 3ebf429c8623c51f944a7245a2e48d2aa088dec4c4914b40aa6049e89856c1ee8586f6e2e3b65195190566637a33004468b51a781e61a082248748015167569b
|
|
|
|
(un)confirmed
fac816544317cee6553d60cb0f5f24f6f9ec98de Remove unused checkFinalTx (MarcoFalke)
fa272eab44553df9b0bed3ca20caf2a7bd193680 wallet: Avoid dropping confirmed coins (MarcoFalke)
888841ea8d38fc059ca06ef67af7f31f8d8418a4 interfaces: Remove unused is_final (MarcoFalke)
dddd05e7a3389fcbd90bb4acdfe1f59945d9f381 qt: Treat unconfirmed txs as unconfirmed (MarcoFalke)
Pull request description:
The wallet has several issues:
## Unconfirmed txs in the GUI
The GUI clumsily attempts to guess if unconfirmed txs are locked until a future time. This is currently based on the locktime only, not nSequence, thus wrong. Fix this by removing the clumsy code and treat all unconfirmed txs as unconfirmed. The GUI already prints whether a tx is in the mempool, in which case the user knows that the tx wasn't locked until a future time. If the tx is not in the mempool, it might be better to report the exact reject reason from the mempool instead of using incorrect heuristics.
## Confirmed txs in the wallet
The wallet drops coins that it incorrectly assumes to be locked until a future time, even if they are already confirmed in the chain. This is because the wallet is using the wrong time (adjusted network time) instead of MTP, due to the `-1` default argument of `CheckFinalTx`.
The issues are fixed in separate commits and there is even a test.
ACKs for top commit:
achow101:
ACK fac816544317cee6553d60cb0f5f24f6f9ec98de
prayank23:
reACK https://github.com/bitcoin/bitcoin/pull/24067/commits/fac816544317cee6553d60cb0f5f24f6f9ec98de
glozow:
code review ACK fac8165443, I understand now how this fixes both issues.
Tree-SHA512: 210afb855f4c6d903fee49eba6b1a9735d699cf0168b669eabb38178e53b3a522258b7cc669f52489c6cd3e38bf358afde12eef3ba2e2f2ffaeb06b8f652ccd0
|
|
|
|
|
|
Coin selection requires knowing the weight of a transaction so that fees
can be estimated. However for external inputs, the weight may not be
avialble, and solving data may not be enough as the input could be one
that we do not support. By allowing users to specify input weights,
those external inputs can be included in the transaction.
Additionally, if the weight for an input is specified, that value will
always be used, regardless of whether the input is in the wallet or
solving data is available. This allows us to account for scenarios where
the wallet may be more conservative and estimate a larger input than may
actually be created.
For example, we assume the maximum DER signature size, but an external
input may be signed by a wallet which does nonce grinding in order to get
a smaller signature. In that case, the user can specify the smaller
input weight to avoid overpaying transaction fees.
|
|
If the user specifies an input size, allow it to override any input size
calculations during coin selection.
|
|
In order to allow coin selection to take weights from the user,
CCoinControl needs to be able to set and get them.
|
|
|
|
This reverts commit d04566415e16ae685af066384f346dff522c068f.
|
|
Refactors TopUp so that it also tops up inactive chains. The bulk of
TopUp is moved to TopUpChain.
CHDChain also has 2 new in memory variables to track its highest used
indexes. This is used only for inactive hd chains so that they can be
topped up later in the same session (e.g. if the wallet is encrypted and
not unlocked at the time of MarkUnusedAddresses).
|
|
|
|
`node::` and `wallet::` namespaces
e5b6aef61221b621ad77b5f075a16897e08835bf Move CBlockFileInfo::ToString method where class is declared (Russell Yanofsky)
f7086fd8ff084ab0dd656d75b7485e59263bdfd8 Add src/wallet/* code to wallet:: namespace (Russell Yanofsky)
90fc8b089d591cabff60ee829a33f96c37fd27ba Add src/node/* code to node:: namespace (Russell Yanofsky)
Pull request description:
There are no code changes, this is just adding `namespace` and `using` declarations and `node::` or `wallet::` qualifiers in some places.
Motivations for this change are:
- To make it easier to see when node and wallet code is being accessed places where it shouldn't be. For example if GUI code is accessing node and wallet internals or if wallet and node code are referencing each other.
- To make source code organization clearer ([#15732](https://github.com/bitcoin/bitcoin/issues/15732)), being able to know that `wallet::` code is in `src/wallet/`, `node::` code is in `src/node/`, `init::` code is in `src/init/`, `util::` code is in `src/util/`, etc.
Reviewing with `git log -p -n1 -U0 --word-diff-regex=.` can be helpful to verify this is only updating declarations, not changing code.
ACKs for top commit:
achow101:
ACK e5b6aef61221b621ad77b5f075a16897e08835bf
MarcoFalke:
Concept ACK e5b6aef61221b621ad77b5f075a16897e08835bf 🍨
Tree-SHA512: 3797745c90246794e2d55a2ee6e8b0ad5c811e4e03a242d3fdfeb68032f8787f0d48ed4097f6b7730f540220c0af99ef423cd9dbe7f76b2ec12e769a757a2c8d
|
|
copy_file calls
3a45dc36a663ea67f13e7d5c413518b9385ec594 Change type of `backup_file` parameter in RestoreWallet/restoreWallet (Hennadii Stepanov)
213172c7348091794b173c2edf5f7f3bbe010912 refactor: Block unsafe std::string fs::path conversion copy_file calls (Hennadii Stepanov)
Pull request description:
This PR is an optional prerequisite for bitcoin/bitcoin#20744 "Use std::filesystem. Remove Boost Filesystem & System" which:
- makes further code changes safer
- prevents [some](https://cirrus-ci.com/task/6525835388649472) test failures on native Windows
ACKs for top commit:
ryanofsky:
Code review ACK 3a45dc36a663ea67f13e7d5c413518b9385ec594. Looks great! Thanks for debugging and fixing this and making #20744 smaller!
Tree-SHA512: c6dfaef6b45b9c268bc9ee9b943b9d679152c9d565ca4f86da8d33f8eb9b3cdbe9ba6df7b7578eacc0d00db6551048beff97419f86eb4b1d3182c43e2b4eb9a5
|
|
`fs::path` looks more native than `std::string` for a parameter which
represents a backup file. This change eliminates back-and-forth type
conversions.
|
|
There is no change in behavior. This just helps prepare for the
transition from boost::filesystem to std::filesystem by avoiding
copy_file calls that will be unsafe after the transition to
std::filesystem to due lack of a boost::filesystem::path::imbue
equivalent and inability to set a predictable locale.
|
|
fa8e01a5b8c67fcb8acaaf58a17ed24710415b6c doc: Remove outdated scriptChange TODO comment (MarcoFalke)
Pull request description:
This was added in commit bf798734db4539a39edd6badf54a1c3aecf193e5 (raw multisig). Raw multisig isn't a thing, so remove the TODO.
ACKs for top commit:
S3RK:
ACK fa8e01a5b8c67fcb8acaaf58a17ed24710415b6c
achow101:
ACK fa8e01a5b8c67fcb8acaaf58a17ed24710415b6c
Tree-SHA512: 01d521ca3605ab130c43531da4922ea85461ca5e7436267a34fb5df348009e086b3c66d85532c62255d9a0ba43db56424884808e773d0ef0177035dfb25d6a6c
|
|
BlockManager
fa68a6c2fc6754c160e0f98007785602201b3c47 scripted-diff: Rename touched member variables (MarcoFalke)
facd3df21f344dd84e5f28862056700c1fded17c Make blockstorage globals private members of BlockManager (MarcoFalke)
faa8c2d7d75f8d9782709e73e00e35851e233392 doc: Clarify nPruneAfterHeight for signet (MarcoFalke)
fad381b2f8e1beb18f748fbeb820e63545b9b0fd test: Load genesis block to allow flush (MarcoFalke)
fab262174b96854d2df5bee7da578990c9e9cb1e Move blockstorage-related unload to BlockManager::Unload (MarcoFalke)
fa467f3913918701c765f9bc754203b4591b894f move-only: Create WriteBlockIndexDB helper (MarcoFalke)
fa88cfd3f9896d5b56ea6c111a23f90a79253c18 Move functions to BlockManager (MarcoFalke)
Pull request description:
Globals aren't too nice because they hide dependencies, also they make testing harder.
Fix that by removing some.
ACKs for top commit:
Sjors:
ACK fa68a6c2fc6754c160e0f98007785602201b3c47
ryanofsky:
Code review ACK fa68a6c2fc6754c160e0f98007785602201b3c47. Nice changes!
Tree-SHA512: 6abc5929a5e43a05e238276721d46a64a44f23dca18c2caa9775437a32351d6815d88b88757254686421531d0df13861bbd3a202e13a3192798d87a96abef65d
|
|
|
|
|
|
the correct size
ac617cc141fe05bea0dc5e8f9df3da43c0945842 wallettool: Check that the dumpfile checksum is the correct size (Andrew Chow)
Pull request description:
After parsing the checksum, make sure that it is the size that we expect it to be.
This issue was reported by Pedro Baptista.
ACKs for top commit:
laanwj:
Code review ACK ac617cc141fe05bea0dc5e8f9df3da43c0945842
Tree-SHA512: 8135b3fb1f4f6b6c91cfbac7d1d3421f1f6c664a742c92940f68eae857f92ce49d042cc3aa5c2df6ef182825271483d65efc7543ec7a8ff047fd7c08666c8899
|
|
Needed for a later commit
|
|
This switches .read() and .write() to take spans of bytes.
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
* 2020: fa0074e2d82928016a43ca408717154a1c70a4db
* 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
fa993d0e7e4d27ca1cf7d88a5a6ec5b93b7ed503 doc: Fix -changetype help text (MarcoFalke)
Pull request description:
This was forgotten in commit 3ac38058ce0e80a9f4276bfa82951decdb237e9a
ACKs for top commit:
shaavan:
ACK fa993d0e7e4d27ca1cf7d88a5a6ec5b93b7ed503
w0xlt:
ACK fa993d0
josibake:
ACK https://github.com/bitcoin/bitcoin/pull/23840/commits/fa993d0e7e4d27ca1cf7d88a5a6ec5b93b7ed503
Tree-SHA512: 9f84b1168e3b3ab06e5c1f4915a1874598b273099eb5878ed28c3a66f1484e34c836fd3c68c4131bee541f3428052f6b66e02b192170752d1082de689d44cd4d
|
|
|
|
Fixed in commit 9522b53a91f28032c34b94662d50b000534708ce
|
|
After parsing the checksum, make sure that it is the size that we expect
it to be.
|
|
Name has been confusing since it was introduced, and it was pointed in
recent review club as https://bitcoincore.reviews/10102 that it was
particularly unclear how interfaces::WalletClient was different from
interfaces::Wallet.
-BEGIN VERIFY SCRIPT-
ren() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; }
ren WalletClient WalletLoader
ren walletClient walletLoader
ren wallet_client wallet_loader
ren "wallet clients release the wallet" "wallet pointer owners release the wallet"
ren "wallet client" "wallet loader"
ren "Wallet client" "Wallet loader"
-END VERIFY SCRIPT-
|
|
This was forgotten in commit 3ac38058ce0e80a9f4276bfa82951decdb237e9a
|
|
|