🚀 fanquake merged a pull request: "multiprocess compatibility updates"
(https://github.com/bitcoin/bitcoin/pull/28721)
(https://github.com/bitcoin/bitcoin/pull/28721)
🤔 Sjors reviewed a pull request: "script, assumeutxo: Enhance validations in utxo_snapshot.sh"
(https://github.com/bitcoin/bitcoin/pull/28852#pullrequestreview-1727230446)
A few suggestions after testing and reviewing 3cb390f3c59d3ed0475f1be0c886d46a044fa898.
(https://github.com/bitcoin/bitcoin/pull/28852#pullrequestreview-1727230446)
A few suggestions after testing and reviewing 3cb390f3c59d3ed0475f1be0c886d46a044fa898.
💬 Sjors commented on pull request "script, assumeutxo: Enhance validations in utxo_snapshot.sh":
(https://github.com/bitcoin/bitcoin/pull/28852#discussion_r1391059644)
On macOS 13.6 this silently exists with `-1` when a node is _not_ pruned (`pruneheight` is not set).
(https://github.com/bitcoin/bitcoin/pull/28852#discussion_r1391059644)
On macOS 13.6 this silently exists with `-1` when a node is _not_ pruned (`pruneheight` is not set).
💬 Sjors commented on pull request "script, assumeutxo: Enhance validations in utxo_snapshot.sh":
(https://github.com/bitcoin/bitcoin/pull/28852#discussion_r1391061396)
If I abort here with `ctrl + c` it tries to cleanup, so maybe move the `trap` calls down a bit?
(https://github.com/bitcoin/bitcoin/pull/28852#discussion_r1391061396)
If I abort here with `ctrl + c` it tries to cleanup, so maybe move the `trap` calls down a bit?
💬 Sjors commented on pull request "script, assumeutxo: Enhance validations in utxo_snapshot.sh":
(https://github.com/bitcoin/bitcoin/pull/28852#discussion_r1391067752)
Note that once `invalidateblock` has been called, this won't stop it. It will go all the way back and then forward again.
(https://github.com/bitcoin/bitcoin/pull/28852#discussion_r1391067752)
Note that once `invalidateblock` has been called, this won't stop it. It will go all the way back and then forward again.
💬 Sjors commented on pull request "script, assumeutxo: Enhance validations in utxo_snapshot.sh":
(https://github.com/bitcoin/bitcoin/pull/28852#issuecomment-1808097606)
By the way, at some point when a bash script gets very complicated, it may be better to convert it to Python.
(https://github.com/bitcoin/bitcoin/pull/28852#issuecomment-1808097606)
By the way, at some point when a bash script gets very complicated, it may be better to convert it to Python.
💬 willcl-ark commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1808117267)
> I think it's human readable? For machines we could just give the raw hex script and not bother with an asm format.
This has always been my interpretation too.
> Anyway, how about going for something more left field...
Square brackets is something I could get behind; to me it is clear from your example that the brackets are indicating _something_, and just by looking at them I can see in most cases that they're hex values. To use the previous (less common?) example formatted with the b
...
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1808117267)
> I think it's human readable? For machines we could just give the raw hex script and not bother with an asm format.
This has always been my interpretation too.
> Anyway, how about going for something more left field...
Square brackets is something I could get behind; to me it is clear from your example that the brackets are indicating _something_, and just by looking at them I can see in most cases that they're hex values. To use the previous (less common?) example formatted with the b
...
💬 theStack commented on pull request "test: refactor: use built-in collection types for type hints (Python 3.9 / PEP 585)":
(https://github.com/bitcoin/bitcoin/pull/28725#issuecomment-1808124759)
> do we have a min supported python version for this project?
Yes, the minimum supported version is documented in `doc/dependencies.md`:
https://github.com/bitcoin/bitcoin/blob/29c2c903621f7daae26113dd2902c016b56929d4/doc/dependencies.md?plain=1#L13
The bump to 3.9 happened a few weeks ago, see https://github.com/bitcoin/bitcoin/pull/28211.
(https://github.com/bitcoin/bitcoin/pull/28725#issuecomment-1808124759)
> do we have a min supported python version for this project?
Yes, the minimum supported version is documented in `doc/dependencies.md`:
https://github.com/bitcoin/bitcoin/blob/29c2c903621f7daae26113dd2902c016b56929d4/doc/dependencies.md?plain=1#L13
The bump to 3.9 happened a few weeks ago, see https://github.com/bitcoin/bitcoin/pull/28211.
💬 hebasto commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1808131170)
> python-altgraph (0.17.4) has been upstreamed...
@achow101 Can you confirm that `python-signapple` is still working as expected?
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1808131170)
> python-altgraph (0.17.4) has been upstreamed...
@achow101 Can you confirm that `python-signapple` is still working as expected?
💬 sipa commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808131524)
@Riahiamirreza You cannot modify `CTxIn` as that's the actual data structure for transaction inputs. If you change it, all attempts to decode transactions will fail.
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808131524)
@Riahiamirreza You cannot modify `CTxIn` as that's the actual data structure for transaction inputs. If you change it, all attempts to decode transactions will fail.
💬 fanquake commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1808154445)
> Can you confirm that python-signapple is still working as expected?
I'm not sure why there would be any issue, given this is a subdependency, of something we don't even use in signapple any more (so only installing has to succeed), and it can ultimtely be removed (I will open PRs).
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1808154445)
> Can you confirm that python-signapple is still working as expected?
I'm not sure why there would be any issue, given this is a subdependency, of something we don't even use in signapple any more (so only installing has to succeed), and it can ultimtely be removed (I will open PRs).
💬 TheCharlatan commented on pull request "test, refactor: Magic bytes array followup":
(https://github.com/bitcoin/bitcoin/pull/28857#discussion_r1391109286)
Thanks, pushed.
(https://github.com/bitcoin/bitcoin/pull/28857#discussion_r1391109286)
Thanks, pushed.
👋 TheCharlatan's pull request is ready for review: "test, refactor: Magic bytes array followup"
(https://github.com/bitcoin/bitcoin/pull/28857)
(https://github.com/bitcoin/bitcoin/pull/28857)
💬 maflcko commented on pull request "log: torcontrol opt checks":
(https://github.com/bitcoin/bitcoin/pull/28780#issuecomment-1808161461)
Are you still working on this?
(https://github.com/bitcoin/bitcoin/pull/28780#issuecomment-1808161461)
Are you still working on this?
💬 maflcko commented on pull request "net: Attempts to connect to all resolved addresses on `addnode`":
(https://github.com/bitcoin/bitcoin/pull/28834#issuecomment-1808162078)
CI failed
(https://github.com/bitcoin/bitcoin/pull/28834#issuecomment-1808162078)
CI failed
💬 vasild commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1391086424)
I think there is an off-by-one error here, should this be `>=` instead of `>`? Lets say the limited peer is at height `H` then the intention of this is to request blocks `(H-286, H]` from it, a total of `286` blocks. However it would not `continue` for height `H-286` and would request that block too.
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1391086424)
I think there is an off-by-one error here, should this be `>=` instead of `>`? Lets say the limited peer is at height `H` then the intention of this is to request blocks `(H-286, H]` from it, a total of `286` blocks. However it would not `continue` for height `H-286` and would request that block too.
💬 vasild commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1391104472)
The full node is at height `1` and the pruned node is at height `289`. The first loop verifies that the full node has blocks `[4, 289]` (`286` blocks in total), which is ok. But then it only checks that the full node does not have block `2`. What about block `3`? It is not checked and should not have been downloaded but it has been downloaded.
To avoid the gap the range in the second loop should be `range(1, limited_threshold_buffer + 1)` or even:
```python
for height in range(start_heigh
...
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1391104472)
The full node is at height `1` and the pruned node is at height `289`. The first loop verifies that the full node has blocks `[4, 289]` (`286` blocks in total), which is ok. But then it only checks that the full node does not have block `2`. What about block `3`? It is not checked and should not have been downloaded but it has been downloaded.
To avoid the gap the range in the second loop should be `range(1, limited_threshold_buffer + 1)` or even:
```python
for height in range(start_heigh
...
📝 fanquake opened a pull request: "[WIP] guix: update signapple (drop macho & altgraph)"
(https://github.com/bitcoin/bitcoin/pull/28859)
Requires https://github.com/achow101/signapple/pull/13. Pointing at my signapple fork if anyone wants to try a build.
(https://github.com/bitcoin/bitcoin/pull/28859)
Requires https://github.com/achow101/signapple/pull/13. Pointing at my signapple fork if anyone wants to try a build.
💬 fanquake commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1808167881)
See https://github.com/achow101/signapple/pull/13 and https://github.com/bitcoin/bitcoin/pull/28859.
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1808167881)
See https://github.com/achow101/signapple/pull/13 and https://github.com/bitcoin/bitcoin/pull/28859.
✅ maflcko closed a pull request: "wallet: clarify replace fields in help output"
(https://github.com/bitcoin/bitcoin/pull/27782)
(https://github.com/bitcoin/bitcoin/pull/27782)