💬 QureshiFaisal commented on pull request "test: refactor: introduce and use `calculate_input_weight` helper":
(https://github.com/bitcoin/bitcoin/pull/29777#issuecomment-2049926127)
tACK [6d91cb7](https://github.com/bitcoin/bitcoin/pull/29777/commits/6d91cb781c30966963f28e7577c7aa3829fa9390)
Functional tests passed for `rpc_psbt.py` and `wallet_send.py`
(https://github.com/bitcoin/bitcoin/pull/29777#issuecomment-2049926127)
tACK [6d91cb7](https://github.com/bitcoin/bitcoin/pull/29777/commits/6d91cb781c30966963f28e7577c7aa3829fa9390)
Functional tests passed for `rpc_psbt.py` and `wallet_send.py`
📝 maflcko opened a pull request: "ci: Bump s390x to ubuntu:24.04"
(https://github.com/bitcoin/bitcoin/pull/29856)
Now that most other CI tasks are rolled to 24.04, roll this one as well.
(https://github.com/bitcoin/bitcoin/pull/29856)
Now that most other CI tasks are rolled to 24.04, roll this one as well.
💬 laanwj commented on pull request "ci: Bump s390x to ubuntu:24.04":
(https://github.com/bitcoin/bitcoin/pull/29856#issuecomment-2049992762)
im not sure it was intended to be rollling from debian to ubuntu too :smile:
Not sure ubuntu exists for s390, but we'll see if CI passes.
(https://github.com/bitcoin/bitcoin/pull/29856#issuecomment-2049992762)
im not sure it was intended to be rollling from debian to ubuntu too :smile:
Not sure ubuntu exists for s390, but we'll see if CI passes.
💬 laanwj commented on pull request "guix: remove `gcc-toolchain static` from Windows build":
(https://github.com/bitcoin/bitcoin/pull/29828#issuecomment-2050005385)
As this still passes the symbol import check (no extra DLL dependencies introduced) i suppose this is correct. Good find!
(https://github.com/bitcoin/bitcoin/pull/29828#issuecomment-2050005385)
As this still passes the symbol import check (no extra DLL dependencies introduced) i suppose this is correct. Good find!
💬 maflcko commented on pull request "ci: Bump s390x to ubuntu:24.04":
(https://github.com/bitcoin/bitcoin/pull/29856#issuecomment-2050041563)
> Not sure ubuntu exists for s390, but we'll see if CI passes.
I think they are only missing riscv64, after commit https://github.com/docker-library/docs/commit/54617b9e5298e9b8325c59081dcd9bd30cb229d6#diff-aa2475d4f8253db9a16955859cf75ba0a5d4f2e2418db0d07390376ca47f1e48R39
(https://github.com/bitcoin/bitcoin/pull/29856#issuecomment-2050041563)
> Not sure ubuntu exists for s390, but we'll see if CI passes.
I think they are only missing riscv64, after commit https://github.com/docker-library/docs/commit/54617b9e5298e9b8325c59081dcd9bd30cb229d6#diff-aa2475d4f8253db9a16955859cf75ba0a5d4f2e2418db0d07390376ca47f1e48R39
⚠️ ChrisCho-H opened an issue: "Enable sighash default option for non-taproot(or tapscript) spend?"
(https://github.com/bitcoin/bitcoin/issues/29857)
### Please describe the feature you'd like to see added.
Why does not enable sighash default for non-taproot transaction(i.e. p2pkh, p2sh, p2wpkh, p2wsh)?
I think it's great BIP341 adds sighash default to save a single byte, which could be huge in aggregate.
As segwit v0 and legacy transactions still take a lot of portion, wouldn't it be good to enable sighash default for them?
It's more efficient, and increases overall backward-compatiblity(without concern about malleability in my humble op
...
(https://github.com/bitcoin/bitcoin/issues/29857)
### Please describe the feature you'd like to see added.
Why does not enable sighash default for non-taproot transaction(i.e. p2pkh, p2sh, p2wpkh, p2wsh)?
I think it's great BIP341 adds sighash default to save a single byte, which could be huge in aggregate.
As segwit v0 and legacy transactions still take a lot of portion, wouldn't it be good to enable sighash default for them?
It's more efficient, and increases overall backward-compatiblity(without concern about malleability in my humble op
...
💬 sipa commented on issue "Enable sighash default option for non-taproot(or tapscript) spend?":
(https://github.com/bitcoin/bitcoin/issues/29857#issuecomment-2050074696)
That's not possible without a hardforking consensus change.
(https://github.com/bitcoin/bitcoin/issues/29857#issuecomment-2050074696)
That's not possible without a hardforking consensus change.
✅ achow101 closed an issue: "Enable sighash default option for non-taproot(or tapscript) spend?"
(https://github.com/bitcoin/bitcoin/issues/29857)
(https://github.com/bitcoin/bitcoin/issues/29857)
💬 emc99 commented on issue "ci: failure in `rpc_scanblocks.py`":
(https://github.com/bitcoin/bitcoin/issues/29831#issuecomment-2050125202)
Who performs ci?
(https://github.com/bitcoin/bitcoin/issues/29831#issuecomment-2050125202)
Who performs ci?
💬 emc99 commented on pull request "ci: Bump s390x to ubuntu:24.04":
(https://github.com/bitcoin/bitcoin/pull/29856#issuecomment-2050172934)
What's s390x?
(https://github.com/bitcoin/bitcoin/pull/29856#issuecomment-2050172934)
What's s390x?
📝 naiyoma opened a pull request: "Test/rpc whitelistdefault test"
(https://github.com/bitcoin/bitcoin/pull/29858)
This PR adds tests for `rpcwhitelistdefault.` The implementation is a continuation of this [PR](https://github.com/bitcoin/bitcoin/pull/17805).
Applied suggestions to include the tests in` rpc_whitelist.py` and to use a single node.
(https://github.com/bitcoin/bitcoin/pull/29858)
This PR adds tests for `rpcwhitelistdefault.` The implementation is a continuation of this [PR](https://github.com/bitcoin/bitcoin/pull/17805).
Applied suggestions to include the tests in` rpc_whitelist.py` and to use a single node.
💬 itornaza commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#issuecomment-2050304085)
tested re-ACK for 858fa78637041ae704005d4b6e564dd8245f4b5d
## No size restrictions
- Did a code review and checked that there is adequate space on the file system to run the tests
- Configured with `--with-incompatible-bdb` and `--enable-suppress-external-warnings`
- Ran unit tests with `make check` and all tests pass
- Ran all functional tests with no extra flags, all tests pass and no warnings about free space are displayed
- Ran all functional tests with `--extended` and all tests p
...
(https://github.com/bitcoin/bitcoin/pull/29335#issuecomment-2050304085)
tested re-ACK for 858fa78637041ae704005d4b6e564dd8245f4b5d
## No size restrictions
- Did a code review and checked that there is adequate space on the file system to run the tests
- Configured with `--with-incompatible-bdb` and `--enable-suppress-external-warnings`
- Ran unit tests with `make check` and all tests pass
- Ran all functional tests with no extra flags, all tests pass and no warnings about free space are displayed
- Ran all functional tests with `--extended` and all tests p
...
💬 laanwj commented on pull request "ci: Bump s390x to ubuntu:24.04":
(https://github.com/bitcoin/bitcoin/pull/29856#issuecomment-2050404404)
> I think they are only missing riscv64, after commit
Great!
> What's s390x?
s390x is some old IBM CPU architecture, we test on it to check if everything works on big-endian machines.
(https://github.com/bitcoin/bitcoin/pull/29856#issuecomment-2050404404)
> I think they are only missing riscv64, after commit
Great!
> What's s390x?
s390x is some old IBM CPU architecture, we test on it to check if everything works on big-endian machines.
💬 davidgumberg commented on pull request "logging: Update to new logging API":
(https://github.com/bitcoin/bitcoin/pull/29231#discussion_r1561932572)
```suggestion
LogWarning("Relative datadir option '%s' specified, which will be interpreted relative to the "
```
(https://github.com/bitcoin/bitcoin/pull/29231#discussion_r1561932572)
```suggestion
LogWarning("Relative datadir option '%s' specified, which will be interpreted relative to the "
```
💬 davidgumberg commented on pull request "logging: Update to new logging API":
(https://github.com/bitcoin/bitcoin/pull/29231#issuecomment-2050850514)
utACK https://github.com/bitcoin/bitcoin/commit/8d46bd6750b47caaf2556f915800c6a22c997bc3
Probably tangential to this PR:
Is `LogPrintLevel` in `void libevent_log_cb(...)` an example of:
> "when [...] a category needs to be added for an info/warning/error log message"
<details>
<summary>Otherwise:</summary>
```diff
static void libevent_log_cb(int severity, const char *msg)
{
- BCLog::Level level;
switch (severity) {
case EVENT_LOG_DEBUG:
- level = BCLo
...
(https://github.com/bitcoin/bitcoin/pull/29231#issuecomment-2050850514)
utACK https://github.com/bitcoin/bitcoin/commit/8d46bd6750b47caaf2556f915800c6a22c997bc3
Probably tangential to this PR:
Is `LogPrintLevel` in `void libevent_log_cb(...)` an example of:
> "when [...] a category needs to be added for an info/warning/error log message"
<details>
<summary>Otherwise:</summary>
```diff
static void libevent_log_cb(int severity, const char *msg)
{
- BCLog::Level level;
switch (severity) {
case EVENT_LOG_DEBUG:
- level = BCLo
...
📝 pablomartin4btc opened a pull request: "Bugfix - don't allow multiple dialogs for same tx in TransactionView"
(https://github.com/bitcoin-core/gui/pull/817)
Limit to one the transaction details dialogs that a user can open.
<details>
<summary>Currently a user can open unlimited tx details dialogs for the same tx id.</summary>

</details>
<details>
<summary>This PR fixes the issue.</summary>

</details>
(https://github.com/bitcoin-core/gui/pull/817)
Limit to one the transaction details dialogs that a user can open.
<details>
<summary>Currently a user can open unlimited tx details dialogs for the same tx id.</summary>

</details>
<details>
<summary>This PR fixes the issue.</summary>

</details>
💬 maflcko commented on pull request "refactor/test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1562077424)
Please no style changes of this kind
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1562077424)
Please no style changes of this kind
👍 maflcko approved a pull request: "test: refactor: introduce and use `calculate_input_weight` helper"
(https://github.com/bitcoin/bitcoin/pull/29777#pullrequestreview-1995987988)
lgtm
(https://github.com/bitcoin/bitcoin/pull/29777#pullrequestreview-1995987988)
lgtm
💬 paplorinc commented on pull request "refactor/test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1562143566)
Reverted.
I've changed it originally to be in alignment with the `EncodeBase64`
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1562143566)
Reverted.
I've changed it originally to be in alignment with the `EncodeBase64`
📝 hebasto opened a pull request: "build: Fix false positive `CHECK_ATOMIC` test for clang-15"
(https://github.com/bitcoin/bitcoin/pull/29859)
On the master branch @ 0de63b8b46eff5cda85b4950062703324ba65a80, a building `bitcoind` with clang-15 for `i686-pc-linux-gnu` fails to link:
```
CXXLD bitcoind
/usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `std::remove_volatile<double>::type std::__atomic_impl::load<double>(double const*, std::memory_order)':
/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:948: undefined reference to `__atomic_load'
/usr/bin/ld: libbitc
...
(https://github.com/bitcoin/bitcoin/pull/29859)
On the master branch @ 0de63b8b46eff5cda85b4950062703324ba65a80, a building `bitcoind` with clang-15 for `i686-pc-linux-gnu` fails to link:
```
CXXLD bitcoind
/usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `std::remove_volatile<double>::type std::__atomic_impl::load<double>(double const*, std::memory_order)':
/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:948: undefined reference to `__atomic_load'
/usr/bin/ld: libbitc
...