🤔 Xyvern26 reviewed a pull request: "doc: upgrade bitcoin core license to 2025."
(https://github.com/bitcoin/bitcoin/pull/31605#pullrequestreview-2531147258)
Danke
(https://github.com/bitcoin/bitcoin/pull/31605#pullrequestreview-2531147258)
Danke
📝 MarcelMonde opened a pull request: "ANEW.Wall"
(https://github.com/bitcoin/bitcoin/pull/31607)
<!--
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly: trading
* Any test improvements or new tests that improve coverage are always welcome.
* All other changes shou
...
(https://github.com/bitcoin/bitcoin/pull/31607)
<!--
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly: trading
* Any test improvements or new tests that improve coverage are always welcome.
* All other changes shou
...
🤔 MarcelMonde reviewed a pull request: "ANEW.Wall"
(https://github.com/bitcoin/bitcoin/pull/31607#pullrequestreview-2531313008)
Let's End Mental Illness, by paying off our debts and preventing a recession! -Marcell of America
(https://github.com/bitcoin/bitcoin/pull/31607#pullrequestreview-2531313008)
Let's End Mental Illness, by paying off our debts and preventing a recession! -Marcell of America
🤔 MarcelMonde reviewed a pull request: "ANEW.Wall"
(https://github.com/bitcoin/bitcoin/pull/31607#pullrequestreview-2531315531)
APPROVE
(https://github.com/bitcoin/bitcoin/pull/31607#pullrequestreview-2531315531)
APPROVE
✅ willcl-ark closed a pull request: "ANEW.Wall"
(https://github.com/bitcoin/bitcoin/pull/31607)
(https://github.com/bitcoin/bitcoin/pull/31607)
💬 maflcko commented on pull request "doc: upgrade bitcoin core license to 2025.":
(https://github.com/bitcoin/bitcoin/pull/31605#issuecomment-2572523772)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
(https://github.com/bitcoin/bitcoin/pull/31605#issuecomment-2572523772)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
💬 bigspider commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2572540702)
> Thanks @bigspider , we’ll take a look.
>
> Our main concern would be compatibility with other wallets. Do you know what wallets support BIP-388 right now (besides Ledger)? Does Sparrow support it?
BIP-388 is the base for the miniscript implementation used currently in Ledger, BitBox and Jade.
Sparrow only supports the standard multisig types, and I suppose it works with most devices.
The `musig` part of BIP-388 is currently only implemented in Ledger in a test application (details [h
...
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2572540702)
> Thanks @bigspider , we’ll take a look.
>
> Our main concern would be compatibility with other wallets. Do you know what wallets support BIP-388 right now (besides Ledger)? Does Sparrow support it?
BIP-388 is the base for the miniscript implementation used currently in Ledger, BitBox and Jade.
Sparrow only supports the standard multisig types, and I suppose it works with most devices.
The `musig` part of BIP-388 is currently only implemented in Ledger in a test application (details [h
...
💬 maflcko commented on pull request "test: fix typo in mempool_ephemeral_dust":
(https://github.com/bitcoin/bitcoin/pull/31604#issuecomment-2572569354)
lgtm ACK 29bca9713d21916b315c2ca0e9183bf567f39351
(https://github.com/bitcoin/bitcoin/pull/31604#issuecomment-2572569354)
lgtm ACK 29bca9713d21916b315c2ca0e9183bf567f39351
💬 maflcko commented on pull request "contrib: fix BUILDDIR in gen-bitcoin-conf script":
(https://github.com/bitcoin/bitcoin/pull/31332#issuecomment-2572608343)
https://github.com/bitcoin/bitcoin/pull/31332#issuecomment-2492544118 is pretty clear that the current docs and script is correct. If the script is changed, the docs will have to be adjusted as well. Otherwise there will be confusion and bugs. So any of the following can be done:
* Close this pull and leave it as-is
* Change the docs *and* the script
* A third alternative could be to make the script a template (`.in` file) and have cmake inject the source and build dir, putting the resultin
...
(https://github.com/bitcoin/bitcoin/pull/31332#issuecomment-2572608343)
https://github.com/bitcoin/bitcoin/pull/31332#issuecomment-2492544118 is pretty clear that the current docs and script is correct. If the script is changed, the docs will have to be adjusted as well. Otherwise there will be confusion and bugs. So any of the following can be done:
* Close this pull and leave it as-is
* Change the docs *and* the script
* A third alternative could be to make the script a template (`.in` file) and have cmake inject the source and build dir, putting the resultin
...
💬 Sjors commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1903873459)
Fixed
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1903873459)
Fixed
📝 maflcko opened a pull request: "doc: Clarify min macOS and Xcode version"
(https://github.com/bitcoin/bitcoin/pull/31608)
Two minor doc fixups:
* Clarify that `macOS 13.0+` means `macOS 13+`, indicating that on any major version, only the latest security release is supported.
* Clarify that the Xcode version was selected based on the minimum required macOS version and the minimum required clang version.
(https://github.com/bitcoin/bitcoin/pull/31608)
Two minor doc fixups:
* Clarify that `macOS 13.0+` means `macOS 13+`, indicating that on any major version, only the latest security release is supported.
* Clarify that the Xcode version was selected based on the minimum required macOS version and the minimum required clang version.
💬 maflcko commented on pull request "ci: Switch to latest macOS and Windows images":
(https://github.com/bitcoin/bitcoin/pull/31597#issuecomment-2572636769)
> Mind picking this up?
Sure, see https://github.com/bitcoin/bitcoin/pull/31608
(https://github.com/bitcoin/bitcoin/pull/31597#issuecomment-2572636769)
> Mind picking this up?
Sure, see https://github.com/bitcoin/bitcoin/pull/31608
💬 Sjors commented on pull request "rpc: fix mintime field testnet4, expand timewarp attack test":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2572654857)
> not clear from the description what the griefing attack is that it seeks to prevent.
It doesn't prevent it, it merely demonstrates. It can only be prevented if the miner actually uses `mintime` or `curtime`, or if a consensus change to increase `MAX_TIMEWARP` to well above `MAX_FUTURE_BLOCK_TIME`.
I expanded the comment.
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2572654857)
> not clear from the description what the griefing attack is that it seeks to prevent.
It doesn't prevent it, it merely demonstrates. It can only be prevented if the miner actually uses `mintime` or `curtime`, or if a consensus change to increase `MAX_TIMEWARP` to well above `MAX_FUTURE_BLOCK_TIME`.
I expanded the comment.
💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2572662414)
Maybe we should have a wallet or `importdescriptors` flag that restricts imported descriptors to BIP-388? Whether to make that the default would be a separate debate.
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2572662414)
Maybe we should have a wallet or `importdescriptors` flag that restricts imported descriptors to BIP-388? Whether to make that the default would be a separate debate.
💬 laanwj commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#issuecomment-2572695215)
See also #31420 which tried to achieve a similar thing by adding conversion functions.
(https://github.com/bitcoin/bitcoin/pull/31595#issuecomment-2572695215)
See also #31420 which tried to achieve a similar thing by adding conversion functions.
👍 TheCharlatan approved a pull request: "doc: Clarify comments about endianness after #30526"
(https://github.com/bitcoin/bitcoin/pull/31596#pullrequestreview-2531711723)
ACK 3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40
(https://github.com/bitcoin/bitcoin/pull/31596#pullrequestreview-2531711723)
ACK 3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40
💬 TheCharlatan commented on pull request "doc: Clarify comments about endianness after #30526":
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1903895648)
Nit: Call it `hexadecimal` instead of `hex` like done in the `"data"` field for better consistency?
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1903895648)
Nit: Call it `hexadecimal` instead of `hex` like done in the `"data"` field for better consistency?
👍 laanwj approved a pull request: "refactor: modernize recent `ByteType` usages and read/write functions"
(https://github.com/bitcoin/bitcoin/pull/31601#pullrequestreview-2531751093)
Code review ACK 9908ab0581ec7a873514a09edb27a7cbaba3611d
Good to get rid of a few redundant `UCharCasts`.
(https://github.com/bitcoin/bitcoin/pull/31601#pullrequestreview-2531751093)
Code review ACK 9908ab0581ec7a873514a09edb27a7cbaba3611d
Good to get rid of a few redundant `UCharCasts`.
👍 Sjors approved a pull request: "doc: Clarify comments about endianness after #30526"
(https://github.com/bitcoin/bitcoin/pull/31596#pullrequestreview-2531755996)
ACK 3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40
(https://github.com/bitcoin/bitcoin/pull/31596#pullrequestreview-2531755996)
ACK 3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40
💬 hodlinator commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#issuecomment-2572740076)
re-ACK f919d919eb8425ef2bb25aa0ebf61c90ab9b07fa
Only commit message improvements since last ACK.
---
PR summary nit:
> Added dedicated padding tests to hardcode the failure behavior.
Not sure "hardcode" is the best verb there, how about:
"Added dedicated padding tests to cover failure behavior."?
(https://github.com/bitcoin/bitcoin/pull/30746#issuecomment-2572740076)
re-ACK f919d919eb8425ef2bb25aa0ebf61c90ab9b07fa
Only commit message improvements since last ACK.
---
PR summary nit:
> Added dedicated padding tests to hardcode the failure behavior.
Not sure "hardcode" is the best verb there, how about:
"Added dedicated padding tests to cover failure behavior."?