Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 instagibbs commented on pull request "test: fix pushdata scripts":
(https://github.com/bitcoin/bitcoin/pull/32270#discussion_r2063676228)
done
💬 instagibbs commented on pull request "test: fix pushdata scripts":
(https://github.com/bitcoin/bitcoin/pull/32270#discussion_r2063676614)
changed to `push` everywhere
📝 9AeinBagheri9 opened a pull request: "Update SECURITY.md"
(https://github.com/bitcoin/bitcoin/pull/32362)
<!--
*** 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
...
👍 rkrux approved a pull request: "test: Force named args for RPCOverloadWrapper optional args"
(https://github.com/bitcoin/bitcoin/pull/32360#pullrequestreview-2799342118)
tACK fa48be3ba443d2436f754265b86331f84b866130

This pull makes the optional looking arguments as named args instead of being positional in the 4 wrapped methods of `RPCOverloadWrapper` class in the functional test framework. I, too, have a preference for named args over positional as I feel it makes the function call sites easier on the eyes.

There are couple removals of unused fields and methods of `RPCOverloadWrapper` class. Some formatting changes are also there that distracts a bit but
...
💬 rkrux commented on pull request "test: Force named args for RPCOverloadWrapper optional args":
(https://github.com/bitcoin/bitcoin/pull/32360#discussion_r2063682773)
Note: This named arg here is passed down to the non-wrapped methods of `RPCOverloadWrapper` class as well because those are present in `rpc_calls` list, but _after_ the positional args.
💬 9AeinBagheri9 commented on pull request "Update SECURITY.md":
(https://github.com/bitcoin/bitcoin/pull/32362#issuecomment-2835286354)
From f116ff8d5839933ee717f99de11b0aa3f05bdef5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=D8=A2=D8=A6=DB=8C=D9=86=20=D8=A8=D8=A7=D9=82=D8=B1=DB=8C?=
=?UTF-8?q?=20=D8=AC=D9=88=D8=B1=D8=A2=D8=A8=DB=8C?=
<bagheriaein18@gmail.com>
Date: Mon, 28 Apr 2025 17:09:48 +0330
Subject: [PATCH] Update SECURITY.md

---
SECURITY.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/SECURITY.md b/SECURITY.md
index fd4c61d176dc3..64d0b49e0abc1 100644
--- a/SECURITY.md
+++ b/SECURIT
...
💬 9AeinBagheri9 commented on pull request "Update SECURITY.md":
(https://github.com/bitcoin/bitcoin/pull/32362#issuecomment-2835288579)
diff --git a/SECURITY.md b/SECURITY.md
index fd4c61d176dc3..64d0b49e0abc1 100644
--- a/SECURITY.md
+++ b/SECURITY.md
@@ -1,4 +1,4 @@
-# Security Policy
+ # Security Policy

## Supported Versions
fanquake closed a pull request: "Update SECURITY.md"
(https://github.com/bitcoin/bitcoin/pull/32362)
📝 fanquake locked a pull request: "Update SECURITY.md"
(https://github.com/bitcoin/bitcoin/pull/32362)
<!--
*** 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
...
📝 GarmashAlex opened a pull request: "ci: Use previous releases in tests on Windows"
(https://github.com/bitcoin/bitcoin/pull/32363)
Currently, none of the CI jobs running on Windows (Windows native and Windows test cross-built) use previous releases for testing. This is causing problems with tests like `wallet_migration.py` which require previous releases to be available.

This PR fixes issue #32192 by implementing the following changes:

1. Added Windows binaries to the SHA256_SUMS dictionary in get_previous_releases.py
2. Modified the check_host function to recognize Windows hosts and map them to the "win64" platform
...
📝 hebasto reopened a pull request: "ci: Use previous releases in tests on Windows"
(https://github.com/bitcoin/bitcoin/pull/32363)
Currently, none of the CI jobs running on Windows (Windows native and Windows test cross-built) use previous releases for testing. This is causing problems with tests like `wallet_migration.py` which require previous releases to be available.

This PR fixes issue #32192 by implementing the following changes:

1. Added Windows binaries to the SHA256_SUMS dictionary in get_previous_releases.py
2. Modified the check_host function to recognize Windows hosts and map them to the "win64" platform
...
🤔 Sjors reviewed a pull request: "Remove arbitrary limits on OP_Return (datacarrier) outputs"
(https://github.com/bitcoin/bitcoin/pull/32359#pullrequestreview-2799281057)
cd7872ca543d31d20f419fd2203138b8301c2e68 looks good other than what I said above: we should make sure the push-only standardness rule is both tested and illustrated. See inline suggestions.

This PR removes more (test) code than I expected, which is good.
💬 Sjors commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2063718969)
```cpp
// Until v30 OP_RETURN was limited to 83 bytes (80 bytes of data, +1 for
// OP_RETURN, +2 for the pushdata opcodes). Add one more byte.
```
💬 Sjors commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2063646275)
I found myself confused why this restart is still needed. It's because the previous test used `-persistmempool=0` and we need to forget about the transaction it created.

```diff
diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py
index cbb1cf789e..85c0f38291 100755
--- a/test/functional/mining_basic.py
+++ b/test/functional/mining_basic.py
@@ -186,6 +186,9 @@ class MiningTest(BitcoinTestFramework):
assert tx_below_min_feerate['txid'] not in block_
...
💬 Sjors commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2063714504)
```diff
diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py
index f4b21cd204..609d826a6f 100755
--- a/test/functional/mempool_accept.py
+++ b/test/functional/mempool_accept.py
@@ -327,6 +327,26 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
rawtxs=[tx.serialize().hex()],
)

+ # OP_RETURN followed by non-push
+ tx = tx_from_hex(raw_tx_reference)
+ tx.vout[0].scriptPubKey = CScript([OP_RETURN, OP_HASH160])
...
💬 l0rinc commented on pull request "test: avoid stack overflow in `FindChallenges` via manual iteration":
(https://github.com/bitcoin/bitcoin/pull/32351#issuecomment-2835394763)
> Refactoring changes, such as introducing the switch statement, could be added in a separate commit

Good idea, done in latest push.
I have also checked https://corecheck.dev/bitcoin/bitcoin/pulls/32351 and saw that Sonar was also compaining about https://sonarcloud.io/organizations/aureleoules/rules?open=cpp%3AS4998&rule_key=cpp%3AS4998, addressed it also in a separate commit.
💬 maflcko commented on pull request "ci: Use previous releases in tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/32363#discussion_r2063756157)
no-op?
💬 maflcko commented on pull request "ci: Use previous releases in tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/32363#discussion_r2063748613)
not sure about running the test separately and then excluding it separately below. Just remove the exclusion?
💬 maflcko commented on pull request "ci: Use previous releases in tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/32363#discussion_r2063753216)
I don't think any wallet files are opened in this test, let alone without a context manager. Why would this be needed?
🤔 sipa reviewed a pull request: "test: avoid stack overflow in `FindChallenges` via manual iteration"
(https://github.com/bitcoin/bitcoin/pull/32351#pullrequestreview-2799484506)
utACK 7e8ef959d0637ca5543ed33d3919937e0d053e70