📝 fanquake locked a pull request: "Add files via upload"
(https://github.com/bitcoin/bitcoin/pull/31341)
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 improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors sh
...
(https://github.com/bitcoin/bitcoin/pull/31341)
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 improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors sh
...
💬 Sjors commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1852357156)
6993836cb6488afa48922f19778ecef6d122bb69 nit: adding these log messages could be its own commit to de-obfuscate what it's actually testing.
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1852357156)
6993836cb6488afa48922f19778ecef6d122bb69 nit: adding these log messages could be its own commit to de-obfuscate what it's actually testing.
💬 Parkeragan commented on issue "When opening or autoloading wallets there should be clear messages about rescanning in progress and wallets' names.":
(https://github.com/bitcoin-core/gui/issues/259#issuecomment-2491683772)
Get wallet type
(https://github.com/bitcoin-core/gui/issues/259#issuecomment-2491683772)
Get wallet type
💬 jobeirne-nydig commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2491695788)
Thanks for the review, all. 3 ACKs so far - any notes from maintainers?
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2491695788)
Thanks for the review, all. 3 ACKs so far - any notes from maintainers?
👍 Sjors approved a pull request: "rpc: Remove submitblock pre-checks"
(https://github.com/bitcoin/bitcoin/pull/31175#pullrequestreview-2452017538)
utACK 563278b6a125f1a3d2111d4ebd74b9cc16d83ec5
It would be good to test the new pruned node behavior, in a way similar to `rpc_getblockfrompeer.py`.
(https://github.com/bitcoin/bitcoin/pull/31175#pullrequestreview-2452017538)
utACK 563278b6a125f1a3d2111d4ebd74b9cc16d83ec5
It would be good to test the new pruned node behavior, in a way similar to `rpc_getblockfrompeer.py`.
💬 Sjors commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1852467087)
3c6eeaf39e01a599829648ecd34990540835962f: even if it was previously pruned.
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1852467087)
3c6eeaf39e01a599829648ecd34990540835962f: even if it was previously pruned.
🤔 instagibbs reviewed a pull request: "test: add missing segwitv1 test cases to `script_standard_tests`"
(https://github.com/bitcoin/bitcoin/pull/31340#pullrequestreview-2451932627)
how about
```
diff --git a/src/addresstype.h b/src/addresstype.h
index 93cdf66c5b..09f3063c61 100644
--- a/src/addresstype.h
+++ b/src/addresstype.h
@@ -115,14 +115,17 @@ public:
if (w1.GetWitnessVersion() > w2.GetWitnessVersion()) return false;
return w1.GetWitnessProgram() < w2.GetWitnessProgram();
}
};
+/** Witness program for Pay-to-Anchor output script type */
+static const std::vector<unsigned char> anchor_bytes{0x4e, 0x73};
+
struct PayToAnchor
...
(https://github.com/bitcoin/bitcoin/pull/31340#pullrequestreview-2451932627)
how about
```
diff --git a/src/addresstype.h b/src/addresstype.h
index 93cdf66c5b..09f3063c61 100644
--- a/src/addresstype.h
+++ b/src/addresstype.h
@@ -115,14 +115,17 @@ public:
if (w1.GetWitnessVersion() > w2.GetWitnessVersion()) return false;
return w1.GetWitnessProgram() < w2.GetWitnessProgram();
}
};
+/** Witness program for Pay-to-Anchor output script type */
+static const std::vector<unsigned char> anchor_bytes{0x4e, 0x73};
+
struct PayToAnchor
...
💬 instagibbs commented on pull request "test: add missing segwitv1 test cases to `script_standard_tests`":
(https://github.com/bitcoin/bitcoin/pull/31340#discussion_r1852412865)
nit: this would be "undefined" rather than "incorrect" like the segwitv0 case
(https://github.com/bitcoin/bitcoin/pull/31340#discussion_r1852412865)
nit: this would be "undefined" rather than "incorrect" like the segwitv0 case
💬 theStack commented on pull request "test: add missing segwitv1 test cases to `script_standard_tests`":
(https://github.com/bitcoin/bitcoin/pull/31340#discussion_r1852555732)
Reworked the comments to match the pattern used in previous cases, which is
`// TxoutType::INTENDED_TYPE with <some minor change that leads to different solver outcome>`
(the comment _"WITNESS_UNKNOWN with incorrect program size"_ didn't make any sense imho)
I kept the "incorrect" term, but wrote the outcome explanation in parantheses. Open for suggestions.
(https://github.com/bitcoin/bitcoin/pull/31340#discussion_r1852555732)
Reworked the comments to match the pattern used in previous cases, which is
`// TxoutType::INTENDED_TYPE with <some minor change that leads to different solver outcome>`
(the comment _"WITNESS_UNKNOWN with incorrect program size"_ didn't make any sense imho)
I kept the "incorrect" term, but wrote the outcome explanation in parantheses. Open for suggestions.
💬 theStack commented on pull request "test: add missing segwitv1 test cases to `script_standard_tests`":
(https://github.com/bitcoin/bitcoin/pull/31340#issuecomment-2491828622)
> how about
> ```
> ...
> ```
> which should get rid of all instances of 0x4e, 0x73?
@instagibbs: Thanks, taken!
> Could also move it into script/script.h and make it a static member of CScript?
Kept as-is for minimal diff, but happy to also do that change if others feel strongly about it.
(https://github.com/bitcoin/bitcoin/pull/31340#issuecomment-2491828622)
> how about
> ```
> ...
> ```
> which should get rid of all instances of 0x4e, 0x73?
@instagibbs: Thanks, taken!
> Could also move it into script/script.h and make it a static member of CScript?
Kept as-is for minimal diff, but happy to also do that change if others feel strongly about it.
💬 instagibbs commented on pull request "test: add missing segwitv1 test cases to `script_standard_tests`":
(https://github.com/bitcoin/bitcoin/pull/31340#issuecomment-2491852015)
ACK https://github.com/bitcoin/bitcoin/pull/31340/commits/a3a4d199e298a76725d0c0424195ae54c7e95fe0
(https://github.com/bitcoin/bitcoin/pull/31340#issuecomment-2491852015)
ACK https://github.com/bitcoin/bitcoin/pull/31340/commits/a3a4d199e298a76725d0c0424195ae54c7e95fe0
💬 instagibbs commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#issuecomment-2491873388)
minimal test case for pruning, feel free to take: https://gist.github.com/instagibbs/cb1f7cfbd0a70e8b5393e457c7a959e6
(https://github.com/bitcoin/bitcoin/pull/31175#issuecomment-2491873388)
minimal test case for pruning, feel free to take: https://gist.github.com/instagibbs/cb1f7cfbd0a70e8b5393e457c7a959e6
💬 jonatack commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2491945225)
> Enforce this in the CI, having it to detect non-local traffic and fail accordingly.
Concept ACK
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2491945225)
> Enforce this in the CI, having it to detect non-local traffic and fail accordingly.
Concept ACK
🤔 BrandonOdiwuor reviewed a pull request: "contrib: fix BUILDDIR in gen-bitcoin-conf script"
(https://github.com/bitcoin/bitcoin/pull/31332#pullrequestreview-2452391378)
tACK b3a96b35691f3fbf958958056cd905e119fb48fe
(https://github.com/bitcoin/bitcoin/pull/31332#pullrequestreview-2452391378)
tACK b3a96b35691f3fbf958958056cd905e119fb48fe
💬 instagibbs commented on pull request "test: Deduplicate assert_mempool_contents()":
(https://github.com/bitcoin/bitcoin/pull/31338#issuecomment-2492087451)
ACK a0eafc10f94362408f54195ffd5a9237dc1ef638
thanks for opening this
(https://github.com/bitcoin/bitcoin/pull/31338#issuecomment-2492087451)
ACK a0eafc10f94362408f54195ffd5a9237dc1ef638
thanks for opening this
🤔 jonatack reviewed a pull request: "contrib: fix BUILDDIR in gen-bitcoin-conf script"
(https://github.com/bitcoin/bitcoin/pull/31332#pullrequestreview-2452455980)
Tested ACK b3a96b35691f3fbf958958056cd905e119fb48fe
With respect to #31161, the bug can be fixed now with this change and that pull easily updated if it moves forward.
(https://github.com/bitcoin/bitcoin/pull/31332#pullrequestreview-2452455980)
Tested ACK b3a96b35691f3fbf958958056cd905e119fb48fe
With respect to #31161, the bug can be fixed now with this change and that pull easily updated if it moves forward.
💬 hebasto commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1852773687)
> TODO:
>
> * [ ] fix `unknown target CPU 'armv8-a+crc+crypto'` [failure](https://cirrus-ci.com/task/5887765378760704) and re-enable macOS cross
Similar to https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2483394112, I do think that `-DWITH_MULTIPROCESS=OFF` can be dropped now as depends are [fixed](https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1848171448).
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1852773687)
> TODO:
>
> * [ ] fix `unknown target CPU 'armv8-a+crc+crypto'` [failure](https://cirrus-ci.com/task/5887765378760704) and re-enable macOS cross
Similar to https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2483394112, I do think that `-DWITH_MULTIPROCESS=OFF` can be dropped now as depends are [fixed](https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1848171448).
💬 mzumsande commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2492157450)
> Honestly, I forgot which test was that.
Maybe it was https://github.com/bitcoin/bitcoin/pull/29605#discussion_r1701616675 (I independently ran into that a few weeks ago and included a fix in #31142)
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2492157450)
> Honestly, I forgot which test was that.
Maybe it was https://github.com/bitcoin/bitcoin/pull/29605#discussion_r1701616675 (I independently ran into that a few weeks ago and included a fix in #31142)
📝 Parkeragan opened a pull request: "Add files via upload"
(https://github.com/bitcoin/bitcoin/pull/31342)
Bitcoin Core has a thorough review process and even the most trivial change
needs to pass a lot of eyes and requires non-zero or even substantial time
effort to review. There is a huge lack of active reviewers on the project, so
patches often sit for a long time.
-->
(https://github.com/bitcoin/bitcoin/pull/31342)
Bitcoin Core has a thorough review process and even the most trivial change
needs to pass a lot of eyes and requires non-zero or even substantial time
effort to review. There is a huge lack of active reviewers on the project, so
patches often sit for a long time.
-->
✅ willcl-ark closed a pull request: "Add files via upload"
(https://github.com/bitcoin/bitcoin/pull/31342)
(https://github.com/bitcoin/bitcoin/pull/31342)