📝 brunoerg opened a pull request: "fuzz: wallet: add target for `TransactionCanBeBumped`"
(https://github.com/bitcoin/bitcoin/pull/33916)
This PR adds a fuzz target for the `TransactionCanBeBumped` function. This function checks if a transaction can be bumped based on:
1. Transaction has descendants in the wallet.
2. Transaction has descendants in the mempool.
3. Transaction has been mined.
4. Transaction was already bumped.
5. Transaction has inputs that are not mine.
This target will create scenarios to exercise all these cases. For the scenario (2), it has a mocked `CTxMemPool` that has a function to update the descen
...
(https://github.com/bitcoin/bitcoin/pull/33916)
This PR adds a fuzz target for the `TransactionCanBeBumped` function. This function checks if a transaction can be bumped based on:
1. Transaction has descendants in the wallet.
2. Transaction has descendants in the mempool.
3. Transaction has been mined.
4. Transaction was already bumped.
5. Transaction has inputs that are not mine.
This target will create scenarios to exercise all these cases. For the scenario (2), it has a mocked `CTxMemPool` that has a function to update the descen
...
⚠️ waketraindev reopened an issue: "No way to clear command history in RPC console or reset the console without restarting the node"
(https://github.com/bitcoin-core/gui/issues/897)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo
- [x] I still think this issue should be opened here
### Report
No way to clear the command history of a RPC console. With the history remaining until client reset.
Please add a way to either clear the command history or fully reset the console.
Thanks in advance
(https://github.com/bitcoin-core/gui/issues/897)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo
- [x] I still think this issue should be opened here
### Report
No way to clear the command history of a RPC console. With the history remaining until client reset.
Please add a way to either clear the command history or fully reset the console.
Thanks in advance
💬 waketraindev commented on issue "No way to clear command history in RPC console or reset the console without restarting the node":
(https://github.com/bitcoin-core/gui/issues/897#issuecomment-3558276507)
A note that some rpc commands like `importdescriptors`, `getdescriptorinfo` do not get filtered or their parameters hidden.
They might contain corrections by the user and linger in the history until shutdown.
A way to fully reset the console without restarting the node seems necessary.
(https://github.com/bitcoin-core/gui/issues/897#issuecomment-3558276507)
A note that some rpc commands like `importdescriptors`, `getdescriptorinfo` do not get filtered or their parameters hidden.
They might contain corrections by the user and linger in the history until shutdown.
A way to fully reset the console without restarting the node seems necessary.
💬 brunoerg commented on pull request "fuzz: wallet: add target for `TransactionCanBeBumped`":
(https://github.com/bitcoin/bitcoin/pull/33916#issuecomment-3558285614)
Coverage report is available at: https://brunoerg.xyz/bitcoin-core-coverage/33916/
(https://github.com/bitcoin/bitcoin/pull/33916#issuecomment-3558285614)
Coverage report is available at: https://brunoerg.xyz/bitcoin-core-coverage/33916/
👍 hebasto approved a pull request: "ci: Enable experimental kernel stuff in most CI tasks via `dev-mode`"
(https://github.com/bitcoin/bitcoin/pull/33824#pullrequestreview-3488101273)
ACK fae83611b8ef358ea7aca7070fd7e82dc06f9755, I have reviewed the code and it looks OK.
(https://github.com/bitcoin/bitcoin/pull/33824#pullrequestreview-3488101273)
ACK fae83611b8ef358ea7aca7070fd7e82dc06f9755, I have reviewed the code and it looks OK.
🚀 hebasto merged a pull request: "ci: Enable experimental kernel stuff in most CI tasks via `dev-mode`"
(https://github.com/bitcoin/bitcoin/pull/33824)
(https://github.com/bitcoin/bitcoin/pull/33824)
🤔 hebasto reviewed a pull request: "depends: update xcb-util packages to latest versions"
(https://github.com/bitcoin/bitcoin/pull/33851#pullrequestreview-3488133192)
My Guix build:
```
aarch64
17cd36aa7eaae8cbe242efa364e55699b1568a6d0cb2c2e1ac82781b46a5f174 guix-build-f541b92cf2bb/output/aarch64-linux-gnu/SHA256SUMS.part
69985b56e1c6ff521f4f49ba102cb53275106dfd076be98361068ef113379fa3 guix-build-f541b92cf2bb/output/aarch64-linux-gnu/bitcoin-f541b92cf2bb-aarch64-linux-gnu-debug.tar.gz
4feb585aaf30ecc8b8c1c1fa32e4a8fa9e7f3e67286825e006807f714f15b039 guix-build-f541b92cf2bb/output/aarch64-linux-gnu/bitcoin-f541b92cf2bb-aarch64-linux-gnu.tar.gz
43b3f41f64a01f
...
(https://github.com/bitcoin/bitcoin/pull/33851#pullrequestreview-3488133192)
My Guix build:
```
aarch64
17cd36aa7eaae8cbe242efa364e55699b1568a6d0cb2c2e1ac82781b46a5f174 guix-build-f541b92cf2bb/output/aarch64-linux-gnu/SHA256SUMS.part
69985b56e1c6ff521f4f49ba102cb53275106dfd076be98361068ef113379fa3 guix-build-f541b92cf2bb/output/aarch64-linux-gnu/bitcoin-f541b92cf2bb-aarch64-linux-gnu-debug.tar.gz
4feb585aaf30ecc8b8c1c1fa32e4a8fa9e7f3e67286825e006807f714f15b039 guix-build-f541b92cf2bb/output/aarch64-linux-gnu/bitcoin-f541b92cf2bb-aarch64-linux-gnu.tar.gz
43b3f41f64a01f
...
🤔 vasild reviewed a pull request: "precalculate SipHash constant salt XORs"
(https://github.com/bitcoin/bitcoin/pull/30442#pullrequestreview-3488156994)
Approach ACK 9f6016c57440041945ace0639d2d97bc87ea6876
(https://github.com/bitcoin/bitcoin/pull/30442#pullrequestreview-3488156994)
Approach ACK 9f6016c57440041945ace0639d2d97bc87ea6876
💬 vasild commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2546366108)
In the commit message of a80521bce064f9a3cf6a349db4b6ee9fd50a4e2f `optimization: Introduce PresaltedSipHasher for repeated hashing`: `build/src/bench/bench_bitcoin` should be changed to `build/bin/bench_bitcoin`.
The tests `SaltedOutpointHasherBench*` do not exist?
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2546366108)
In the commit message of a80521bce064f9a3cf6a349db4b6ee9fd50a4e2f `optimization: Introduce PresaltedSipHasher for repeated hashing`: `build/src/bench/bench_bitcoin` should be changed to `build/bin/bench_bitcoin`.
The tests `SaltedOutpointHasherBench*` do not exist?
💬 vasild commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2546273279)
In `master`, the `shorttxidk0` and `shorttxidk1` members are default initialized to `0` when the object is created. Then they are assigned in `CBlockHeaderAndShortTxIDs::FillShortTxIDSelector()` and eventually used in `CBlockHeaderAndShortTxIDs::GetShortID()`.
This means that (in `master`) a call to `GetShortID()` without a preceding call to `FillShortTxIDSelector()` is well defined and will use `shorttxidk...` as `0`.
Now, with these changes such an usage would trigger this newly added `Asser
...
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2546273279)
In `master`, the `shorttxidk0` and `shorttxidk1` members are default initialized to `0` when the object is created. Then they are assigned in `CBlockHeaderAndShortTxIDs::FillShortTxIDSelector()` and eventually used in `CBlockHeaderAndShortTxIDs::GetShortID()`.
This means that (in `master`) a call to `GetShortID()` without a preceding call to `FillShortTxIDSelector()` is well defined and will use `shorttxidk...` as `0`.
Now, with these changes such an usage would trigger this newly added `Asser
...
💬 vasild commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2546377030)
Would be nice to wrap the commit mesage lines to 72 chars.
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2546377030)
Would be nice to wrap the commit mesage lines to 72 chars.
💬 fjahr commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-3558487859)
Only rebased again
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-3558487859)
Only rebased again
👍 hebasto approved a pull request: "depends: update xcb-util packages to latest versions"
(https://github.com/bitcoin/bitcoin/pull/33851#pullrequestreview-3488345544)
ACK f541b92cf2bb7f8f708f147bd320becf48587d14, due to the last commit, I also tested an LTO build on Ubuntu 25.10 using GCC 15.2.0.
(https://github.com/bitcoin/bitcoin/pull/33851#pullrequestreview-3488345544)
ACK f541b92cf2bb7f8f708f147bd320becf48587d14, due to the last commit, I also tested an LTO build on Ubuntu 25.10 using GCC 15.2.0.
🚀 hebasto merged a pull request: "depends: update xcb-util packages to latest versions"
(https://github.com/bitcoin/bitcoin/pull/33851)
(https://github.com/bitcoin/bitcoin/pull/33851)
🤔 hodlinator reviewed a pull request: "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation"
(https://github.com/bitcoin/bitcoin/pull/33878#pullrequestreview-3486433210)
Reviewed b41f5a29f7d56da8fd157770ad29b5776c3684c6
Sorry for not finding these earlier, getting closer to A-C-K.
(https://github.com/bitcoin/bitcoin/pull/33878#pullrequestreview-3486433210)
Reviewed b41f5a29f7d56da8fd157770ad29b5776c3684c6
Sorry for not finding these earlier, getting closer to A-C-K.
💬 hodlinator commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2545107996)
in 6b9ce8cff58d7214fca51a8024ec4efb8694e059 on `CheckAsmap`:
1. Justification for existence: I dislike default parameter values, so appreciate a distinct function from `SanityCheckASMap`.
2. Comment: Nothing makes it specialized for only embedded data?
3. Location: Maybe it could be added directly after `SanityCheckASMap` in both .h + .cpp since they are so closely related? (Could place it before but that would put it between `Interpret` and `SanityCheckASMap` which are nice to have after eac
...
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2545107996)
in 6b9ce8cff58d7214fca51a8024ec4efb8694e059 on `CheckAsmap`:
1. Justification for existence: I dislike default parameter values, so appreciate a distinct function from `SanityCheckASMap`.
2. Comment: Nothing makes it specialized for only embedded data?
3. Location: Maybe it could be added directly after `SanityCheckASMap` in both .h + .cpp since they are so closely related? (Could place it before but that would put it between `Interpret` and `SanityCheckASMap` which are nice to have after eac
...
💬 hodlinator commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2545022654)
meganit in d278d6193b8d754eb108bc3ccc256260085b32c9 "refactor: Unify asmap version calculation and naming":
As `uint256 AsmapVersion(const std::vector<std::byte>& data)` is new in this PR, it could be introduced as `uint256 AsmapVersion(std::span<const std::byte> data)` from the beginning to avoid line churn.
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2545022654)
meganit in d278d6193b8d754eb108bc3ccc256260085b32c9 "refactor: Unify asmap version calculation and naming":
As `uint256 AsmapVersion(const std::vector<std::byte>& data)` is new in this PR, it could be introduced as `uint256 AsmapVersion(std::span<const std::byte> data)` from the beginning to avoid line churn.
💬 hodlinator commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2546283406)
nit: In other cases we have
* `DecodeBits(size_t& bitpos, const std::span<const std::byte>& data...` and
* `DecodeType(size_t& bitpos, const std::span<const std::byte>& data)`
* ...
So it might be nice to have the same argument order here:
```suggestion
inline bool GetBitLE(uint32_t bitpos, std::span<const std::byte> bytes) noexcept
{
return (std::to_integer<uint8_t>(bytes[bitpos / 8]) >> (bitpos % 8)) & 1;
}
/**
* Extract a single bit from byte array using big-endian bit ord
...
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2546283406)
nit: In other cases we have
* `DecodeBits(size_t& bitpos, const std::span<const std::byte>& data...` and
* `DecodeType(size_t& bitpos, const std::span<const std::byte>& data)`
* ...
So it might be nice to have the same argument order here:
```suggestion
inline bool GetBitLE(uint32_t bitpos, std::span<const std::byte> bytes) noexcept
{
return (std::to_integer<uint8_t>(bytes[bitpos / 8]) >> (bitpos % 8)) & 1;
}
/**
* Extract a single bit from byte array using big-endian bit ord
...
💬 hodlinator commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2546124184)
nit: Here are some changes that bring the `Interpret()` implementation even closer to `SanityCheckAsmap()` while also decreasing variable scope, making the code more readable. Maybe something to include in the last commit which aligns the casing of `SanityCheckAsmap`?
```diff
--- a/src/util/asmap.cpp
+++ b/src/util/asmap.cpp
@@ -183,18 +183,16 @@ uint32_t Interpret(const std::span<const std::byte> asmap, const std::span<const
const size_t endpos{asmap.size() * 8};
uint8_t bits = ip.siz
...
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2546124184)
nit: Here are some changes that bring the `Interpret()` implementation even closer to `SanityCheckAsmap()` while also decreasing variable scope, making the code more readable. Maybe something to include in the last commit which aligns the casing of `SanityCheckAsmap`?
```diff
--- a/src/util/asmap.cpp
+++ b/src/util/asmap.cpp
@@ -183,18 +183,16 @@ uint32_t Interpret(const std::span<const std::byte> asmap, const std::span<const
const size_t endpos{asmap.size() * 8};
uint8_t bits = ip.siz
...
💬 hodlinator commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2544942080)
Thank you! In adfd800626a7910a603c34ab733fc1e91d438bc5 the `ASMAP_DATA` type is still unnecessarily changed from `constexpr auto` to `static const auto`.
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2544942080)
Thank you! In adfd800626a7910a603c34ab733fc1e91d438bc5 the `ASMAP_DATA` type is still unnecessarily changed from `constexpr auto` to `static const auto`.