Age | Commit message (Collapse) | Author |
|
Use std::filesystem::rename() instead of std::rename(). We rely on the
destination to be overwritten if it exists, but std::rename()'s behavior
is implementation-defined in this case.
|
|
strencodings
fac9fe5d051264fcd16e8e36d30f28c05c999837 Fix unintended unsigned integer overflow in strencodings (MarcoFalke)
Pull request description:
This fixes two issues for strings that start with a colon and only have one colon:
* `fMultiColon` is incorrectly set to `true`
* There is an unsigned integer overflow `colon - 1` (`0 - 1`)
Neither issue matters, as the result is discarded. Though, it makes sense to still fix the issue for clarity and to avoid sanitizer issues in the function.
ACKs for top commit:
laanwj:
Code review ACK fac9fe5d051264fcd16e8e36d30f28c05c999837
shaavan:
Code Review ACK fac9fe5d051264fcd16e8e36d30f28c05c999837
Tree-SHA512: e71c21a0b617abf241e561ce6b90b963e2d5e2f77bd9547ce47209a1a94b454384391f86ef5d35fedd4f4df19add3896bb3d61fed396ebba8e864e3eeb75ed59
|
|
FormatParagraph
fa2f7d005932bff9b7d27744ae517b9e7910df8d fuzz: Avoid unsigned integer overflow in FormatParagraph (MarcoFalke)
Pull request description:
`FormatParagraph` is only ever called with compile time constant arguments, so I don't see the need for fuzzing it.
Though, keep it for now, but avoid the unsigned integer overflow with this patch.
ACKs for top commit:
laanwj:
Code review ACK fa2f7d005932bff9b7d27744ae517b9e7910df8d
Tree-SHA512: 01fc64a9ef73c183921ca1b0cd8db9514c0a242e3acf215a3393f383ae129e01625ebb16eaf9cb86370eda62d0145c3dcf8f62e40edf5958abc1f777c5687280
|
|
|
|
|
|
|
|
|
|
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
|
|
fa1b227a727a5056c6fbc7e4f33c19aeb5207718 Remove broken and unused CDataStream methods (MarcoFalke)
faee5f8dc23cd2fcfb6ad62a1d46ad3020ef0c5c test: Create fresh CDataStream each time (MarcoFalke)
fa71114926490e84c9222d315a95684d250e8e34 test: Inline expected_xor (MarcoFalke)
Pull request description:
The `insert` and `erase` methods have many issues:
* They are unused
* They are confusing and hard to read, as they implement "special cases" for optimization, that isn't needed
* They are broken (See https://github.com/bitcoin/bitcoin/pull/24231)
* Fixing them leads to mingw compile errors (See https://github.com/bitcoin/bitcoin/pull/24231#issuecomment-1029286985)
Fix all issues by removing them
ACKs for top commit:
laanwj:
Code review ACK fa1b227a727a5056c6fbc7e4f33c19aeb5207718
Tree-SHA512: 9d9e5d42e6ffc5ae82bdb67cfb5b50b45977ae674acee6ff99092560aebf2fc7e4584ded614e190db0663226fa198e34350517cd7ee57d518de22e9568bc349a
|
|
|
|
|
|
validation.cpp
fac62056b56e0a28baf0b6f285752d83fbf96074 Fix integer sanitizer suppressions in validation.cpp (MarcoFalke)
Pull request description:
It doesn't seem ideal to have an integer sanitizer enabled, but then disable it for the whole validation.cpp file.
Fix it with a refactor and remove the suppression.
ACKs for top commit:
hebasto:
ACK fac62056b56e0a28baf0b6f285752d83fbf96074, I have reviewed the code and it looks OK, I agree it can be merged.
prayank23:
Code Review ACK https://github.com/bitcoin/bitcoin/pull/24196/commits/fac62056b56e0a28baf0b6f285752d83fbf96074
Tree-SHA512: efc5b9887cb2e207033b264ebf425bae5ff013e909701c049aea5d79a21f10495826e962d171b3d412717cbf0a4723e5124133b5401b35a73915212e85e91020
|
|
aeb18b665c616c3326671b4c7e9d6421306564f0 Bugfix: GUI: Check validity when QValidatedLineEdit::setText is called (Luke Dashjr)
b1a544be109d336c0b53722e3f8b51687972c94e Bugfix: GUI: Re-check validity after QValidatedLineEdit::setCheckValidator (Luke Dashjr)
2385b508d5f2db118513c3e0b343d2309cdfdcd8 Bugfix: GUI: Only apply invalid style to QValidatedLineEdit, not its tooltip (Luke Dashjr)
Pull request description:
1. Use a CSS selector to avoid changing the background colour of the tooltip.
2. Re-check validity of input when we first set the validator (probably a no-op in practice).
3. Check validity of input when it is set programmatically via `setText` (eg, via the address book). Probably no-op in practice UNTIL merging https://github.com/bitcoin/bitcoin/pull/15987 or any other PR that adds a warning for valid addresses.
Moved from https://github.com/bitcoin/bitcoin/pull/18133 (just concept ACKs)
ACKs for top commit:
Sjors:
tACK aeb18b665c616c3326671b4c7e9d6421306564f0
hebasto:
ACK aeb18b665c616c3326671b4c7e9d6421306564f0, tested on Linux Mint 20.3 (Qt 5.12.8).
Tree-SHA512: b6fa8ee4dec76e1c759095721240e6fa5071a02993cb28406e96a0fa2e819f5dddc03d2e7c9073354d7865c2b09eb263afaf853ecba42e9fc4f50ef4ae20bf0f
|
|
99de8068cd08ecc2ad5dfe603bf3c2cc5b8b33aa validation: use stronger EXCLUSIVE_LOCKS_REQUIRED() (Vasil Dimov)
Pull request description:
https://github.com/bitcoin/bitcoin/pull/24103 added annotations to
denote that the callers of `CChainState::ActivateBestChain()` and
`CChainState::InvalidateBlock()` must not own `m_chainstate_mutex` at
the time of the call.
Replace the added `LOCKS_EXCLUDED()` with a stronger
`EXCLUSIVE_LOCKS_REQUIRED()`, see
https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#negative for the
difference between both.
ACKs for top commit:
hebasto:
ACK 99de8068cd08ecc2ad5dfe603bf3c2cc5b8b33aa.
jonatack:
ACK 99de8068cd08ecc2ad5dfe603bf3c2cc5b8b33aa. Tested with Debian clang version 13.0.1. Reproduced hebasto's results. Verified that `LoadExternalBlockFile()` needs the annotation added here.
Tree-SHA512: 59640d9ad472cdb5066ecde89cc0aff8632a351fc030f39bb43800d2c856fb1aed3576e4134212d32be161b18780f06dc5066ac71df7f7cd69e3f21f886e1542
|
|
std::filesystem:::create_directories() call
b9c113af754540341d9529532fbadb7525168102 util: Avoid buggy std::filesystem:::create_directories() call (Hennadii Stepanov)
Pull request description:
Compiled with some libstdc++ versions (e.g., on Ubuntu 20.04) [`std::filesystem:::create_directories()`](https://en.cppreference.com/w/cpp/filesystem/create_directory) call [fails](https://github.com/bitcoin/bitcoin/issues/24257#issue-1123753243) to handle symbol links properly.
No behavior change in comparison to the [pre-20744](https://github.com/bitcoin/bitcoin/commit/c194293883fbb656779102309b2cb3e60889feff) master branch.
Fixes bitcoin/bitcoin#24257.
ACKs for top commit:
ryanofsky:
Code review ACK b9c113af754540341d9529532fbadb7525168102. Nice simplification and fix
MarcoFalke:
review ACK b9c113af754540341d9529532fbadb7525168102 🐬
Tree-SHA512: 79d940cfc1f68d9b0548fb2ab005e90850b54ac0fb3bb2940afd632d56288d92687579a3176bac3fd0ea3d2dae71e26444f8f7bdb87862414c12866ae5e857c4
|
|
|
|
fadcd031390dd4588bbb1c07e5020a7131312050 Fix unsigned integer overflow in LoadMempool (MarcoFalke)
Pull request description:
It doesn't seem ideal to have an integer sanitizer enabled, but then disable it for the whole validation.cpp file.
This removes one of the two violations.
This should be a refactor.
ACKs for top commit:
prayank23:
Code Review ACK https://github.com/bitcoin/bitcoin/pull/24227/commits/fadcd031390dd4588bbb1c07e5020a7131312050
Tree-SHA512: 9fb2f3d49008a59cd45b7c17be0c88c04e61183197c11c8176865af5532c8d0c940db49a351dd0fc75e1d7fd8678c3b816d34cfca170dc6b9cf8f37fdf1c8cae
|
|
fad81548fa03861c244397201d6b6e6cbf883c38 test: Avoid testing negative block heights (MarcoFalke)
Pull request description:
A negative chain height is only used to denote an empty chain, not the height of any block.
So stop testing that and remove a suppression.
ACKs for top commit:
brunoerg:
crACK fad81548fa03861c244397201d6b6e6cbf883c38
Tree-SHA512: 0f9e91617dfb6ceda99831e6cf4b4bf0d951054957c159b1a05a178ab6090798fae7368edefe12800da24585bcdf7299ec3534f4d3bbf5ce6a6eca74dd3bb766
|
|
validation weight calculation
fadc54b79b14ba0bbdcf5eff1277295851fe7a9e Fix unsigned integer overflow in tapscript validation weight calculation (MarcoFalke)
Pull request description:
Change the tapscript validation weight constants from uint64_t to int64_t, since the type of m_validation_weight_left is also int64_t. Otherwise this will cause sanitizer warnings.
This should be safe because signed integer overflow isn't expected to happen.
ACKs for top commit:
PastaPastaPasta:
utACK fadc54b79b14ba0bbdcf5eff1277295851fe7a9e
theStack:
Code-review ACK fadc54b79b14ba0bbdcf5eff1277295851fe7a9e
Tree-SHA512: 7a62d3a84733ab7827e3fa50d83f5493f2481b725c587e986eb2c128a769f023756f3ad964401526e386a847aa630a9f6c43a57d25ce5fd4af0b6bb5e0615528
|
|
Compiled with some libstdc++ versions (e.g., on Ubuntu 20.04)
std::filesystem:::create_directories() call fails to handle symbol links
properly.
|
|
d216bc8d76d7f4e9dce58b0bb732a2d4deaf23b6 Re-enable walletinit_verify_walletdir_no_trailing2 test disabled in #20744 (Ryan Ofsky)
80cd64e84296f1166e133c237fa0afc046b01ce2 Re-enable util_datadir check disabled in #20744 (Ryan Ofsky)
Pull request description:
Reenable some broken tests as discussed https://github.com/bitcoin/bitcoin/pull/20744#discussion_r798651736 and https://github.com/bitcoin/bitcoin/pull/20744#discussion_r798678137
Fix windows test cases broken in #20744, by passing normalized path arguments to fs::equivalent, fs::exists, and fs::is_directory, instead of non-normalized arguments. Also re-enable the tests.
It is possible these changes also fix real init behavior on windows when -datadir or -walletdir paths with trailing dots or dashes are used, but it's not clear because I only tested on wine.
ACKs for top commit:
hebasto:
ACK d216bc8d76d7f4e9dce58b0bb732a2d4deaf23b6, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 2099ddfa1a3ad70f7ac2ff413929414a1851d257b280da25c0f5cefb46fd1372b580a1f1ee5280681a1c16e6031f119185cadd4f7a6121298562cf001f711068
|
|
std::string
824e1ffa9fd957d05e34f36abe381c2465d89702 bench: Represents paths with fs::path instead of std::string (Ryan Ofsky)
Pull request description:
Suggested https://github.com/bitcoin/bitcoin/pull/20744#issuecomment-1022486215
ACKs for top commit:
fanquake:
untested ACK 824e1ffa9fd957d05e34f36abe381c2465d89702
hebasto:
ACK 824e1ffa9fd957d05e34f36abe381c2465d89702, tested on Linux Mint 20.2 (x86_64).
Tree-SHA512: 348fc189f30b5ad9a8e49e95e535d2c044462a9d534c3f34d887fbde0c05c41e88e02b4fc340709e6395a1188496a8333eb9e734310aa4c41755ec080e53c06e
|
|
|
|
7eeb778fef Merge sipa/minisketch#58: Move `#ifdef HAVE_CLMUL` guard outside of the EnableClmul definition
4d9db2b897 Move `#ifdef HAVE_CLMUL` guard outside of the EnableClmul definition
git-subtree-dir: src/minisketch
git-subtree-split: 7eeb778fef45e21abca01ede85cf0a82e8a510df
|
|
Also uses fs::path quoting in bench printed strings and fixes a
misleading error message.
Originally suggested https://github.com/bitcoin/bitcoin/pull/20744#issuecomment-1022486215
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
|
|
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.
|
|
This should also fix an assert error if a -datadir 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 fix adds a
path::lexically_normal() call to the failing assert so it passes.
|
|
source
985d85e9a8d3dd6e52fef27f76b49c5aa4b34f30 Follow Transifex docs to prepare XLIFF source (Hennadii Stepanov)
Pull request description:
This PR is a #21694 follow up.
From the Transifex [docs](https://docs.transifex.com/formats/xliff#how-to-distinguish-between-a-source-file-and-a-translation-file):
> A source file is different than a translation file. The translation file contains \<Target> references, whereas a source file does not.
This PR makes the `qt/locale/bitcoin_en.xlf` source file according to the docs.
ACKs for top commit:
laanwj:
ACK 985d85e9a8d3dd6e52fef27f76b49c5aa4b34f30
Tree-SHA512: 537ef78f39a12f094b17fc902c5b59c7ae4d27e5fa35fbf6b33386843e4521236dac3275ba26ff2d1c435e53e8942284644d5de67b0b3543cec96cbcd4351607
|
|
timer->setInterval() argument
f7a19ef774ef92ce348215593e3590a750c345e1 qt,refactor: Use std::chrono in TrafficGraphWidget class (Shashwat)
Pull request description:
The PR is a follow-up to #517
- It addresses the change suggested in [this](https://github.com/bitcoin-core/gui/pull/517#pullrequestreview-850260826) comment.
- This PR changes the type of `msecsPerSample` from **int** to **std::chrono::minutes** and makes other relevant subsequent changes that were limited to the **trafficgraphwidget** file.
ACKs for top commit:
RandyMcMillan:
tACK f7a19ef774ef92ce348215593e3590a750c345e1
hebasto:
ACK f7a19ef774ef92ce348215593e3590a750c345e1
promag:
Code review ACK f7a19ef774ef92ce348215593e3590a750c345e1.
Tree-SHA512: 5094ba894f3051fc99148cb8f408fc6f9d6571188673dcb7bf24366cdfb3eaf6d4e41083685d578ad2a9fbe31cc491a5f3fa9b7c9ab6eb90e4dc1356f89ae18a
|
|
04255073bbd2b8ea71ae8a9ff7433be499312758 qt: Update source translations (laanwj)
cf79c56e65595612d899f0551348656b00d1108f init: Remove confusing '(possible integer overflow?)' from error message (laanwj)
d570a63894d5285d350b628b47014abba280743f qt: Update transifex resource blob to 23.0 (laanwj)
Pull request description:
- Update translations for 0.23 string freeze
- Update transifex resource blob to 23.0
This is necessary before a 23.0 resource can be created on Transifex.
ACKs for top commit:
hebasto:
ACK 04255073bbd2b8ea71ae8a9ff7433be499312758
Tree-SHA512: ff886e92721f070e3c135cfec229c41848a67c02355b88f2a5a507241b545f4209167d83b561420c2a82f49a5994170b01dcfb95bfc3fe6b9c832abcc6547b14
|
|
ef5014d256638735b292672c774446db4003f03b style: wrap long lines in CNode creation and add some comments (Vasil Dimov)
b68349164827f14c472201cad54c4e19a3321261 scripted-diff: rename CNode::cs_hSocket to CNode::m_sock_mutex (Vasil Dimov)
c41a1162ac4da437c5d755e8fe2bf636bed22b0f net: use Sock in CNode (Vasil Dimov)
c5dd72e146dd8fa77d29c8689a42322a4d1ec780 fuzz: move FuzzedSock earlier in src/test/fuzz/util.h (Vasil Dimov)
Pull request description:
_This is a piece of #21878, chopped off to ease review._
Change `CNode` to use a pointer to `Sock` instead of a bare `SOCKET`.
This will help mocking / testing / fuzzing more code.
ACKs for top commit:
jonatack:
re-ACK ef5014d256638735b292672c774446db4003f03b changes since last review are the removal of an unneeded dtor and the addition of a style commit
w0xlt:
reACK ef5014d
PastaPastaPasta:
utACK ef5014d256638735b292672c774446db4003f03b, I have reviewed the code, and believe it makes sense to merge
theStack:
Cod-review ACK ef5014d256638735b292672c774446db4003f03b
Tree-SHA512: 7f5414dd339cd2f16f7cbdc5fcec238d68b6d50072934aea10b901f409da28ff1ece6db6e899196616aa8127b8b25ab5b86d000bdcee58b4cadd7a3c1cf560c5
|
|
|
|
Can be reviewed with --ignore-all-space
|
|
|
|
|
|
|
|
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>
|
|
update the i2p seeds
bcc5676f16d86a2d271e0d822d836d9c47ef298b p2p, contrib: update i2p hardcoded seeds (Jon Atack)
e5332425fc9527645aade0c7509c4c3f72e9fe95 p2p, contrib: add cjdns hardcoded seeds (Jon Atack)
Pull request description:
This update targets the v23 release. Additional reliable CJDNS seeds, and feedback on the I2P seeds, is welcome. To verify `src/chainparamsseeds.h` locally, run:
```
contrib/seeds/generate-seeds.py contrib/seeds > src/chainparamsseeds.h
```
ACKs for top commit:
laanwj:
Code review ACK bcc5676f16d86a2d271e0d822d836d9c47ef298b
lsilva01:
tACK bcc5676
Tree-SHA512: 40b1bbb89b9677e1e88c17ac279d6ff5a8ea9f4a1e1ef07e2b71074471308da4760b69cf5130134082c25e7caf4ded02bcc89bd75fae68c2834c64c230e82ac4
|
|
|
|
fad84a25956ec081f22aebbda309d168a3dc0004 refactor: Fixup uint64_t-cast style in touched line (MarcoFalke)
fa041878de786f5be74ec74a06ec407c99ca8656 Fix implicit-integer-sign-change in bloom (MarcoFalke)
Pull request description:
Signed values don't really make sense when using `std::vector::operator[]`.
Fix that and remove the suppression.
ACKs for top commit:
PastaPastaPasta:
utACK fad84a25956ec081f22aebbda309d168a3dc0004
theStack:
Code-review ACK fad84a25956ec081f22aebbda309d168a3dc0004
Tree-SHA512: 7139dd9aa098c41e4af1b6e63dd80e71a92b0a98062d1676b01fe550ffa8e21a5f84a578afa7a536d70dad1b8a5017625e3a9e2dda6f864b452ec77b130ddf2a
|
|
https://github.com/bitcoin/bitcoin/pull/24103 added annotations to
denote that the callers of `CChainState::ActivateBestChain()` and
`CChainState::InvalidateBlock()` must not own `m_chainstate_mutex` at
the time of the call.
Replace the added `LOCKS_EXCLUDED()` with a stronger
`EXCLUSIVE_LOCKS_REQUIRED()`, see
https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#negative for the
difference between both.
|
|
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
|
|
|
|
|
|
fa2406a50a83184d101d1bb3f2b282ae280370ba zmq: Fix implicit-integer-sign-change (MarcoFalke)
Pull request description:
uint256::begin() returns unsigned data, so there is no reason to make it signed.
Fix that and remove the sanitizer suppression.
ACKs for top commit:
hebasto:
ACK fa2406a50a83184d101d1bb3f2b282ae280370ba
PastaPastaPasta:
utACK fa2406a50a83184d101d1bb3f2b282ae280370ba, I have reviewed the code and think it makes sense
Tree-SHA512: 150ebcf3fdc3e0f60b6fd8e5fe638737b01e8a0863296bd545fb5ed17d33ab23b2ff94204996aa7b4617650b7383bd86ed2d2bf46746b410feae449de179a2bd
|
|
faa630aa15bbda0f3b0cf3b6f31cf8fdaeb66975 test: Fix sanitizer suppresions in streams_tests (MarcoFalke)
Pull request description:
Two changes (that also make sense on their own) to remove the file-wide sanitizer suppression:
* `FindByte` no longer takes a `char`, but an `uint8_t`, after commit 196b4599201dbce3e0317e9b98753fa6a244b82d.
* The `key` vector of unsigned chars can be removed and inlined as initializer-list. This avoids a bunch of verbose code like `clear()` and `push_back` of `char`s.
ACKs for top commit:
PastaPastaPasta:
utACK faa630aa15bbda0f3b0cf3b6f31cf8fdaeb66975, I have reviewed the changes and agree it makes sense to merge
Tree-SHA512: 747b9d4676fad6d07f3955668639c93333625e69199ff4c499f01167de3875990d93db85e775a7f5b1b684575dceaec8aa000b4db15525fc47b699bac1c85e3d
|
|
|
|
|
|
|
|
type unsigned
1111d33532516c16fb2e22660ac2745ce56ad6cd refactor: Make MessageBoxFlags enum underlying type unsigned (MarcoFalke)
Pull request description:
All values in the enum are unsigned. Also, flags shouldn't be treated as signed types. So clarify the underlying type and remove a sanitizer suppression.
ACKs for top commit:
hebasto:
ACK 1111d33532516c16fb2e22660ac2745ce56ad6cd, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 48b16c4a0ace1a1e4d351d6eadadbb1bc42aef7fd82e24e3ea50c62f2c04a552ed21027158d09aa97e630c8c7d732cb150c38065333d7c2accbae46593b7ed9f
|