💬 Sjors commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#issuecomment-2231396250)
I think you lost https://github.com/bitcoin/bitcoin/pull/30046/commits/e11d764a6ce72cd571ad9cf818c6918dd16b02f6 in the rebase?
(https://github.com/bitcoin/bitcoin/pull/28122#issuecomment-2231396250)
I think you lost https://github.com/bitcoin/bitcoin/pull/30046/commits/e11d764a6ce72cd571ad9cf818c6918dd16b02f6 in the rebase?
💬 instagibbs commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679768600)
I'm actually having trouble figuring out where this stuff is exposed since we wouldn't be issuing these addresses as a wallet. Mind if I punt on this?
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679768600)
I'm actually having trouble figuring out where this stuff is exposed since we wouldn't be issuing these addresses as a wallet. Mind if I punt on this?
📝 maflcko opened a pull request: "doc: getaddressinfo[isscript] is optional"
(https://github.com/bitcoin/bitcoin/pull/30457)
`isscript` is unknown for unknown witness versions, so it should be marked optional in the docs
(https://github.com/bitcoin/bitcoin/pull/30457)
`isscript` is unknown for unknown witness versions, so it should be marked optional in the docs
⚠️ maflcko opened an issue: "Fuzz coverage for wallet RPCs is zero"
(https://github.com/bitcoin/bitcoin/issues/30458)
Fuzz coverage for wallet RPCs is zero, however it could help turn up bugs such as https://github.com/bitcoin/bitcoin/issues/30456.
Ref: https://maflcko.github.io/b-c-cov/fuzz.coverage/src/wallet/rpc/addresses.cpp.gcov.html
_Originally posted by @maflcko in https://github.com/bitcoin/bitcoin/issues/30456#issuecomment-2231372884_
(https://github.com/bitcoin/bitcoin/issues/30458)
Fuzz coverage for wallet RPCs is zero, however it could help turn up bugs such as https://github.com/bitcoin/bitcoin/issues/30456.
Ref: https://maflcko.github.io/b-c-cov/fuzz.coverage/src/wallet/rpc/addresses.cpp.gcov.html
_Originally posted by @maflcko in https://github.com/bitcoin/bitcoin/issues/30456#issuecomment-2231372884_
💬 maflcko commented on issue "Fuzz coverage for wallet RPCs is zero":
(https://github.com/bitcoin/bitcoin/issues/30458#issuecomment-2231427446)
I presume this is possible to implement by DRYing some code from the `rpc` fuzz target into a `wallet_rpc` fuzz target.
(https://github.com/bitcoin/bitcoin/issues/30458#issuecomment-2231427446)
I presume this is possible to implement by DRYing some code from the `rpc` fuzz target into a `wallet_rpc` fuzz target.
💬 instagibbs commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679783383)
done
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679783383)
done
💬 instagibbs commented on issue "getaddressinfo: complains missing `isscript` when called on unknown witness version":
(https://github.com/bitcoin/bitcoin/issues/30456#issuecomment-2231435940)
@maflcko feels related to me ;)
(https://github.com/bitcoin/bitcoin/issues/30456#issuecomment-2231435940)
@maflcko feels related to me ;)
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2231438781)
I added e11d764a6ce72cd571ad9cf818c6918dd16b02f6 which seems to fix the mismatch. Comparing the rest now.
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2231438781)
I added e11d764a6ce72cd571ad9cf818c6918dd16b02f6 which seems to fix the mismatch. Comparing the rest now.
💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1679800434)
Fixed. Thanks!
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1679800434)
Fixed. Thanks!
📝 gabrielsellan-paylivre opened a pull request: "Alterando readme"
(https://github.com/bitcoin/bitcoin/pull/30459)
<!--
*** Please remove the following help text before submitting: ***
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:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/30459)
<!--
*** Please remove the following help text before submitting: ***
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:
* Any test improvements or new tests that improv
...
✅ fanquake closed a pull request: "Alterando readme"
(https://github.com/bitcoin/bitcoin/pull/30459)
(https://github.com/bitcoin/bitcoin/pull/30459)
📝 fanquake locked a pull request: "Alterando readme"
(https://github.com/bitcoin/bitcoin/pull/30459)
<!--
*** Please remove the following help text before submitting: ***
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:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/30459)
<!--
*** Please remove the following help text before submitting: ***
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:
* Any test improvements or new tests that improv
...
🤔 fjahr reviewed a pull request: "test: assumeutxo: add missing tests in wallet_assumeutxo.py"
(https://github.com/bitcoin/bitcoin/pull/30455#pullrequestreview-2180929310)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30455#pullrequestreview-2180929310)
Concept ACK
💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1679832936)
You should use the `fastprune` option as well and check if there has already been some pruning happening when the test runs, i.e. check that there is some pruneheight. There isn't much of a difference if the option is set but nothing has actually been pruned yet.
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1679832936)
You should use the `fastprune` option as well and check if there has already been some pruning happening when the test runs, i.e. check that there is some pruneheight. There isn't much of a difference if the option is set but nothing has actually been pruned yet.
💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1679825657)
nit: should be moved up for alphabetical order
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1679825657)
nit: should be moved up for alphabetical order
💬 sipa commented on pull request "random: add benchmarks and drop unnecessary Shuffle function":
(https://github.com/bitcoin/bitcoin/pull/30396#issuecomment-2231512385)
> I hope all code touched in this pull request is covered by fuzz tests. If not, then that should be fixed ;)
Hmm, for some reason I only considered calls to `Shuffle` / `std::shuffle` in the fuzz tests themselves. But you're right, shuffles in the code being tested would also result in fuzz inconsistency between platforms.
(https://github.com/bitcoin/bitcoin/pull/30396#issuecomment-2231512385)
> I hope all code touched in this pull request is covered by fuzz tests. If not, then that should be fixed ;)
Hmm, for some reason I only considered calls to `Shuffle` / `std::shuffle` in the fuzz tests themselves. But you're right, shuffles in the code being tested would also result in fuzz inconsistency between platforms.
💬 maflcko commented on pull request "build: Introduce CMake-based buid system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1679842517)
One can see that they are indeed disabled now in https://corecheck.dev/bitcoin/bitcoin/pulls/30454 (coverage report + benchmarks).
Shouldn't autotools be nuked in this pull request, assuming that cmake and autotools are apparently incompatible, as designed now?
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1679842517)
One can see that they are indeed disabled now in https://corecheck.dev/bitcoin/bitcoin/pulls/30454 (coverage report + benchmarks).
Shouldn't autotools be nuked in this pull request, assuming that cmake and autotools are apparently incompatible, as designed now?
🤔 ryanofsky reviewed a pull request: "logging: Replace LogError and LogWarning with LogAlert"
(https://github.com/bitcoin/bitcoin/pull/30364#pullrequestreview-2180955015)
Rebased d34b529ed104a3a7e11c7d264703e61d84b1aeb3 -> 4e9ff3100ae79f3c3046a3358ff16daa7cd89627 ([`pr/alert.2`](https://github.com/ryanofsky/bitcoin/commits/pr/alert.2) -> [`pr/alert.3`](https://github.com/ryanofsky/bitcoin/commits/pr/alert.3), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/alert.2-rebase..pr/alert.3)) due to conflict with #30428
(https://github.com/bitcoin/bitcoin/pull/30364#pullrequestreview-2180955015)
Rebased d34b529ed104a3a7e11c7d264703e61d84b1aeb3 -> 4e9ff3100ae79f3c3046a3358ff16daa7cd89627 ([`pr/alert.2`](https://github.com/ryanofsky/bitcoin/commits/pr/alert.2) -> [`pr/alert.3`](https://github.com/ryanofsky/bitcoin/commits/pr/alert.3), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/alert.2-rebase..pr/alert.3)) due to conflict with #30428
💬 ryanofsky commented on pull request "logging: Replace LogError and LogWarning with LogAlert":
(https://github.com/bitcoin/bitcoin/pull/30364#discussion_r1679841065)
re: https://github.com/bitcoin/bitcoin/pull/30364#discussion_r1675452625
> I think you can split out the 8 `/d` delete statements and three script lines for a simple last commit to just manually delete those lines
Good idea, done in latest push.
(https://github.com/bitcoin/bitcoin/pull/30364#discussion_r1679841065)
re: https://github.com/bitcoin/bitcoin/pull/30364#discussion_r1675452625
> I think you can split out the 8 `/d` delete statements and three script lines for a simple last commit to just manually delete those lines
Good idea, done in latest push.
👋 maflcko's pull request is ready for review: "test: Non-Shy version sender"
(https://github.com/bitcoin/bitcoin/pull/30453)
(https://github.com/bitcoin/bitcoin/pull/30453)