💬 TheCharlatan commented on pull request "test, refactor: Magic bytes array followup":
(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)

💬 maflcko commented on pull request "log: torcontrol opt checks":
(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

💬 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.

💬 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
...

📝 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.

💬 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.

✅ maflcko closed a pull request: "wallet: clarify replace fields in help output"
(https://github.com/bitcoin/bitcoin/pull/27782)

💬 maflcko commented on pull request "wallet: clarify replace fields in help output":
(https://github.com/bitcoin/bitcoin/pull/27782#issuecomment-1808169132)
Closing as up for grabs for now, due to lack of reply

💬 maflcko commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1808172554)
Could mark as draft while CI is red?

💬 fanquake commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1808174414)
Guix Build (aarch64):
```bash
13a7d1be447ecb614cf43034af4f7a3a7ce7dffbcdb6c1773bc939ba80587ef6 guix-build-92d12f1c8903/output/aarch64-linux-gnu/SHA256SUMS.part
5ab66c69b742a89b0aa52705e48563749cb72ebcc92745a4eb07df285a20c62a guix-build-92d12f1c8903/output/aarch64-linux-gnu/bitcoin-92d12f1c8903-aarch64-linux-gnu-debug.tar.gz
d3224eb0eb66bf4433ed8757667ed438e419db4240b06d76122e8754de241742 guix-build-92d12f1c8903/output/aarch64-linux-gnu/bitcoin-92d12f1c8903-aarch64-linux-gnu.tar.gz
9f5ecd
...

💬 hebasto commented on pull request "[WIP] guix: update signapple (drop macho & altgraph)":
(https://github.com/bitcoin/bitcoin/pull/28859#issuecomment-1808174701)
Concept ACK.

💬 MatthewLM 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-1808180595)
`0x` is much more standard and recognisable than `[]`.

💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808188088)
@sipa Yes and I'm not sure how should I fix the problem. Actually the `redeemScript` is a new field which is not part of the actual data, it's only a new representation of data (decompiled version of `redeemScript`). Would you suggest any approach to fix it?

💬 maflcko commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808190478)
> Would you suggest any approach to fix it?

Fix what? Without any information, there is nothing we can do here.

If you want to fix the tests, make sure to revert the test changes and run the test locally.

💬 vasild commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1808191887)
ACK fca0a8938e34cb4f6c400e1d1d0be02f027d80c5

💬 sipa commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808196658)
I don't see why you'd need to make changes to the test framework at all.

💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808199147)
@sipa @maflcko I added new field to the output of `TxToUniv` function, called `redeemScript`. I've manually tested the rpc and that works fine. After running tests I faced issue of decoding transactions. Because of adding a new field, transactions couldn't be decoded successfully. I attempt to change `CTxIn` message in my next effort and add the `redeemScript` which now seems trivially incorrect way.

💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808204753)
@maflcko Sure, I'll revert my changes on test framework and then try another way to fix tests.