Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 hebasto commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#issuecomment-1665233643)
> > All other comments are to be addressed shortly.
>
> Are you still working on this?

I am. My apologies for a delay.
💬 naumenkogs commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1665244546)
ACK 1b52d16d07be3b5d968157913f04d9cd1e2d3678
💬 naumenkogs commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1284155363)
1e9684f39fba909b3501e9402d5b61f4bf744ff2
"w hether"
👍 naumenkogs approved a pull request: "p2p: Drop m_recently_announced_invs bloom filter"
(https://github.com/bitcoin/bitcoin/pull/27675#pullrequestreview-1562445761)
ACK 1e9684f39fba909b3501e9402d5b61f4bf744ff2
💬 ChrisCho-H commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1284186407)
I understand what you intended but it does not seem to be interpreted in that way for `pop two top stack items`. `pop pop add push` is more confusing to me. I appreciate your feedback
💬 Sjors commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1284194811)
I guess it's not entirely clear to me whose responsibility it is to ensure the handshake has been done: https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1283674053 (I assume it will be in the main PR)
💬 hebasto commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1284195974)
I hope that at some point, GitHub Actions will host `arm64` macOS runners, enabling us to test both architectures.

Maybe leave both files for now?
💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1284198100)
Good call, i've adopted this approach for both the `CBlockFile`s and the `CBlockHeader`s.
💬 Sjors commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1284204363)
Maybe keep it simple and just test 1 of 1000.
💬 Sjors commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1284205838)
Why are you changing the fee rate?
💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1284207518)
Does apple still ship Intel silicon for new macOS machines? Regardless, I don't really see a benefit in running the same task twice when there is no reason to believe it will find more bugs or is even useful in the long term.
💬 Sjors commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1284212036)
I understand descriptors, but Python less well :-) What is this function trying to do?
💬 ChrisCho-H commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1665337473)
Seems no one's against @vostrnad suggestion, I convert all single lines to block comments, add textual description with notation and reference for OP_SUCCESS opcodes in case of tapscript.
I didn't include history much. In my opinion, description only for function itself looks clean and explicit which fits with main purpose of this.
📝 MarcoFalke opened a pull request: "ci: Move tidy and macOS-cross to persistent workers"
(https://github.com/bitcoin/bitcoin/pull/28214)
Cirrus CI will be capping the free compute soon. For now, switch more tasks to persistent worker, as recommended by Cirrus CI.

(See slightly related discussion in https://github.com/bitcoin/bitcoin/issues/28098)

Also, add more docs.
💬 eriknylund commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1284233075)
If I didn't bump the fee rate I would run into the two error codes mentioned in the PR message:
```
Had to bump fee rate to resolve assertions in sendtoaddress and psbt methods:

code -5: assert rpc_online.gettransaction(txid)["confirmations"] > 0
code -26: min relay fee not met
```

The previous `fee_rate = 200` limit seems to be sufficient up until `n ~ 500`. I assume this is because the script size increases with higher numbers of `n`.
💬 hebasto commented on pull request "ci: Move tidy and macOS-cross to persistent workers":
(https://github.com/bitcoin/bitcoin/pull/28214#issuecomment-1665359618)
macOS task has not been started:
![image](https://github.com/bitcoin/bitcoin/assets/32963518/b92441c3-3864-4e52-895e-84e1de39e983)
💬 eriknylund commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1284239102)
I guess that would do the trick, however I'm unsure if by doing so we modify the test to test the assertion, which seems a bit incomplete/unintuitive to me, rather than testing the edge cases in k-of-N wallets? 🤔 If you understand what I mean 😄 If you don't think it's an issue, I'm all for keeping it simple and just test 1-of-1000.
💬 Sjors commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1284246711)
Fee rate is the fee per vbyte. But this may be related to #26573
💬 MarcoFalke commented on pull request "ci: Move tidy and macOS-cross to persistent workers":
(https://github.com/bitcoin/bitcoin/pull/28214#issuecomment-1665375160)
> UPD: Scheduled in 14 minutes:

When there are pushes to many pull requests at the same time, scheduling may take a few minutes. This is similar to the Cirrus CI hosted open source compute cluster.
💬 Sjors commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1284249102)
Well the "edge" here is n = 999, but this test is varying `k`, not `n`.