📝 guilhermelinosp opened a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/30313)
<!--
*** 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/30313)
<!--
*** 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
...
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647632350)
58 minutes for the second run, which is acceptable.
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647632350)
58 minutes for the second run, which is acceptable.
📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/30313)
<!--
*** 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/30313)
<!--
*** 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
...
📝 Sjors opened a pull request: "ci: clarify Cirrus self-hosted workers setup"
(https://github.com/bitcoin/bitcoin/pull/30314)
Taken from #29274 (except for two paragraphs that require the other commits in that PR).
(https://github.com/bitcoin/bitcoin/pull/30314)
Taken from #29274 (except for two paragraphs that require the other commits in that PR).
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2180800535)
@maflcko done in #30314, minus the paragraphs about `NO_ARM` and `NO_BRANCH`.
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2180800535)
@maflcko done in #30314, minus the paragraphs about `NO_ARM` and `NO_BRANCH`.
💬 ryanofsky commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647664881)
In commit "ci: forks can opt-out of CI branch push (Cirrus only)" (1c35e3c47fe814c402d423e247b26cb6da2e960e)
I'm still confused by this. I understand we don't want PR contributors to "initially see all jobs marked as skipped". But what does this cause them to see? Jobs just not showing up at all? Also I don't understand why it says "initially." Would the jobs be only initially skipped and not permanently skipped for some reason?
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647664881)
In commit "ci: forks can opt-out of CI branch push (Cirrus only)" (1c35e3c47fe814c402d423e247b26cb6da2e960e)
I'm still confused by this. I understand we don't want PR contributors to "initially see all jobs marked as skipped". But what does this cause them to see? Jobs just not showing up at all? Also I don't understand why it says "initially." Would the jobs be only initially skipped and not permanently skipped for some reason?
💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1647665176)
I think we similarly make `CTransaction` a pointer to keep the `CScriptCheck` class `is_nothrow_move_assignable_v`.
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1647665176)
I think we similarly make `CTransaction` a pointer to keep the `CScriptCheck` class `is_nothrow_move_assignable_v`.
🤔 maflcko reviewed a pull request: "doc: clarify Cirrus self-hosted workers setup"
(https://github.com/bitcoin/bitcoin/pull/30314#pullrequestreview-2130605205)
ACK 9f4255ac57929de985425f26ad904a12176a0e85 🌂
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 9f4255ac57929de985425f26ad
...
(https://github.com/bitcoin/bitcoin/pull/30314#pullrequestreview-2130605205)
ACK 9f4255ac57929de985425f26ad904a12176a0e85 🌂
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 9f4255ac57929de985425f26ad
...
💬 maflcko commented on pull request "doc: clarify Cirrus self-hosted workers setup":
(https://github.com/bitcoin/bitcoin/pull/30314#discussion_r1647677834)
```suggestion
# The above machine types are matched to each task by their label. Refer to the
```
Cirrus calls it `task`. In yaml it is `task:`
(https://github.com/bitcoin/bitcoin/pull/30314#discussion_r1647677834)
```suggestion
# The above machine types are matched to each task by their label. Refer to the
```
Cirrus calls it `task`. In yaml it is `task:`
💬 maflcko commented on pull request "doc: clarify Cirrus self-hosted workers setup":
(https://github.com/bitcoin/bitcoin/pull/30314#discussion_r1647683866)
nit: empty newline should stay here, because the section ends. you'll have to remove the `#` above instead.
(https://github.com/bitcoin/bitcoin/pull/30314#discussion_r1647683866)
nit: empty newline should stay here, because the section ends. you'll have to remove the `#` above instead.
💬 fanquake commented on pull request "doc: clarify Cirrus self-hosted workers setup":
(https://github.com/bitcoin/bitcoin/pull/30314#discussion_r1647688711)
> which may work out of the box with podman or docker.
Please don't add vaugeries like this to docs. If you have a specific issue that you're running into, please open an issue, otherwise things should be assumed to be working (otherwise they should be fixed, rather than adding docs claiming they might not work).
(https://github.com/bitcoin/bitcoin/pull/30314#discussion_r1647688711)
> which may work out of the box with podman or docker.
Please don't add vaugeries like this to docs. If you have a specific issue that you're running into, please open an issue, otherwise things should be assumed to be working (otherwise they should be fixed, rather than adding docs claiming they might not work).
💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2180871592)
f946b083fe1431fb8d5d2c080ed3c2938116c37a -> 974934b3c23ce7f34ef46f3dfc13468cd46f4b8a ([noGlobalScriptCache_6](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_6) -> [noGlobalScriptCache_7](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_6..noGlobalScriptCache_7))
* Addressed @maflcko's [nit](https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1647123546), removed unused inc
...
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2180871592)
f946b083fe1431fb8d5d2c080ed3c2938116c37a -> 974934b3c23ce7f34ef46f3dfc13468cd46f4b8a ([noGlobalScriptCache_6](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_6) -> [noGlobalScriptCache_7](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_6..noGlobalScriptCache_7))
* Addressed @maflcko's [nit](https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1647123546), removed unused inc
...
💬 maflcko commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1647706071)
nit: You added the headers in one commit, and then removed them in the next. Wrong order?
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1647706071)
nit: You added the headers in one commit, and then removed them in the next. Wrong order?
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1647708122)
Notice this does not apply to all transactions though. The exception here applies to transactions that have some ancestors **in the mempool**. In that case, we need to be consistent with the way we share this set of transactions, otherwise, it could be the case that the parents are reconciled at time T and the children are fanout at T-n, potentially making the children orphan.
Under these conditions, we are checking how many peers have the ancestors in their reconciliation sets, and selecting
...
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1647708122)
Notice this does not apply to all transactions though. The exception here applies to transactions that have some ancestors **in the mempool**. In that case, we need to be consistent with the way we share this set of transactions, otherwise, it could be the case that the parents are reconciled at time T and the children are fanout at T-n, potentially making the children orphan.
Under these conditions, we are checking how many peers have the ancestors in their reconciliation sets, and selecting
...
💬 maflcko commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2180895187)
ACK 974934b3c23ce7f34ef46f3dfc13468cd46f4b8a 🌤
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 974934b3c23ce7f34ef46f3dfc
...
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2180895187)
ACK 974934b3c23ce7f34ef46f3dfc13468cd46f4b8a 🌤
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 974934b3c23ce7f34ef46f3dfc
...
💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1647713760)
They are required in 201b29e468f621a1fb055060920231ef46192414 and then no-longer in 974934b3c23ce7f34ef46f3dfc13468cd46f4b8a.
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1647713760)
They are required in 201b29e468f621a1fb055060920231ef46192414 and then no-longer in 974934b3c23ce7f34ef46f3dfc13468cd46f4b8a.
💬 maflcko commented on pull request "ci: add option for running tests without volume":
(https://github.com/bitcoin/bitcoin/pull/30310#issuecomment-2180953573)
> Is there a reason why we don't want to just have cirrus behave in the same way we'd do it on GHA, seems like GHA cannot store to shared cache but cirrus can act the same was as GHA. Just thinking maybe theres a way to reduce complexity in `ci/test/02_run_container.sh` and have more shared logic between the two ci's
Yes, it is possible to do the same on cirrus. However, for local runs it can not be done, because `DANGER_CI_ON_HOST_CACHE_FOLDERS` is dangerous and bricks your system.
(https://github.com/bitcoin/bitcoin/pull/30310#issuecomment-2180953573)
> Is there a reason why we don't want to just have cirrus behave in the same way we'd do it on GHA, seems like GHA cannot store to shared cache but cirrus can act the same was as GHA. Just thinking maybe theres a way to reduce complexity in `ci/test/02_run_container.sh` and have more shared logic between the two ci's
Yes, it is possible to do the same on cirrus. However, for local runs it can not be done, because `DANGER_CI_ON_HOST_CACHE_FOLDERS` is dangerous and bricks your system.
💬 maflcko commented on pull request "ci: add option for running tests without volume":
(https://github.com/bitcoin/bitcoin/pull/30310#issuecomment-2180959651)
> Maybe wait for the CI to pass, then address the nits to test the created cache?
Oh, I guess only master creates the cache?
```
if: github.event_name != 'pull_request' && steps.ccache-cache.outputs.cache-hit != 'true'
```
So I guess this can be merged after addressing the nits.
(https://github.com/bitcoin/bitcoin/pull/30310#issuecomment-2180959651)
> Maybe wait for the CI to pass, then address the nits to test the created cache?
Oh, I guess only master creates the cache?
```
if: github.event_name != 'pull_request' && steps.ccache-cache.outputs.cache-hit != 'true'
```
So I guess this can be merged after addressing the nits.
📝 Sjors opened a pull request: "Stratum v2 Transport"
(https://github.com/bitcoin/bitcoin/pull/30315)
Based on #29346. Parent PR #29432.
Introduces `Sv2Transport::Transport` which is very similar to `V2Transport`.
TODO:
* shoehorn `Sv2NetMsg` into a `CSerializedNetMsg` in `SetMessageToSend` (currently broken)
* shoehorn `Sv2NetMsg` into a `CNetMessage` in `GetReceivedMessage` (currently works)
* use `BytesToSend` instead of `Sv2BytesToSend`? (currently works)
See discussion in #30209.
The first few commits move some network related code from Node to Common. My goal is to make the
...
(https://github.com/bitcoin/bitcoin/pull/30315)
Based on #29346. Parent PR #29432.
Introduces `Sv2Transport::Transport` which is very similar to `V2Transport`.
TODO:
* shoehorn `Sv2NetMsg` into a `CSerializedNetMsg` in `SetMessageToSend` (currently broken)
* shoehorn `Sv2NetMsg` into a `CNetMessage` in `GetReceivedMessage` (currently works)
* use `BytesToSend` instead of `Sv2BytesToSend`? (currently works)
See discussion in #30209.
The first few commits move some network related code from Node to Common. My goal is to make the
...