Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ maflcko commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2109025763)
nit in https://github.com/bitcoin/bitcoin/commit/2b202e9db56487e658fc41089178f31ef4259a0d: Any reason to remove this here?
πŸ’¬ maflcko commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2109038105)
nit in https://github.com/bitcoin/bitcoin/commit/2b202e9db56487e658fc41089178f31ef4259a0d: could clarify the exact error string?

```
too-long-mempool-chain, too many unconfirmed ancestors [limit: 100]
πŸ’¬ maflcko commented on pull request "test: Add missing ipc subtree to lint":
(https://github.com/bitcoin/bitcoin/pull/32623#discussion_r2109053322)
thx, I'll leave as-is for now to make it possible to review via `--color-moved=dimmed-zebra`. Seems better to be done as a repo-wide follow-up anyway, because this affects all docs.
πŸ’¬ l0rinc commented on pull request "test: Add missing ipc subtree to lint":
(https://github.com/bitcoin/bitcoin/pull/32623#issuecomment-2912331467)
ACK fa2bf6bdb7e5f8940fe4bce1aaab29bf2e063b49
πŸ’¬ maflcko commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-2912362918)
https://cirrus-ci.com/task/4713767570767872?logs=ci#L3220
πŸ’¬ maflcko commented on pull request "fuzz: wallet: add target for `MigrateToDescriptor`":
(https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2109086986)
why is this needed? Isn't the chain already `MAIN`?
πŸ€” TheCharlatan reviewed a pull request: "Add checkBlock() to Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31981#pullrequestreview-2870950393)
Concept ACK
πŸ’¬ TheCharlatan commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2109107660)
In commit 981853ab25dc6f967f393ecba57965afb8a822f3

I would have suggested the same. It is not clear to me why you are introducing a new `blockCoins` (camel case?) view here. Maybe do some of these `TestBlockValidity` refactors and documentation in a separate commit. It is not quite clear to me why this should be changed.
πŸ’¬ maflcko commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2109111837)
β€œinto a OP_RETURN output.”
-> β€œinto an OP_RETURN output.” [article before vowel sound β€œO”]


I am also thinking about splitting up the "make this arg optional". Seems like a small and easy preparatory pull request?
πŸ’¬ maflcko commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2109115291)

participatns -> participants [correct misspelling in error message]
πŸ’¬ brunoerg commented on pull request "fuzz: wallet: add target for `MigrateToDescriptor`":
(https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2109117905)
Just removed it. Redudant.
πŸ’¬ brunoerg commented on pull request "fuzz: wallet: add target for `MigrateToDescriptor`":
(https://github.com/bitcoin/bitcoin/pull/32624#issuecomment-2912418699)
08b574dde2c5f13a4c9dadcb9d8bded158c59623..5f2fb4dd333f8d1db27c78414afc84454e6ee4a5: Address https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2109086986
πŸ€” naiyoma reviewed a pull request: "rpc: generateblock to allow multiple outputs"
(https://github.com/bitcoin/bitcoin/pull/32468#pullrequestreview-2870982878)

A summary of my understanding of this PR so far, regarding the `generateblock` changes: :
- [x] Outputs and transactions are now optional.
- [x] `generateblock` can now take an array of addresses and distribute the reward equally among them.
- [x] `generateblock` with an empty array - in this case fallback to OP_TRUE
- [ ] I think there's an intention to add a remainder β€” that way, it's possible to specify part of the reward to specific addresses and then split the remainder among the oth
...
πŸ’¬ maflcko commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109129254)
https://cirrus-ci.com/task/4825274484785152?logs=ci#L3624

```
in sync_mempools
[23:01:19.556] raise AssertionError("Mempool sync timed out after {}s:{}".format(
[23:01:19.556] AssertionError: Mempool sync timed out after 2400s:
[23:01:19.556]
πŸ’¬ instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2109129595)
weight / WITNESS_SCALE_FACTOR = vbytes

A bit confusing
πŸ’¬ naiyoma commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2109130072)
Delete this instead of commenting it out.
πŸ’¬ maflcko commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109130959)

that size -> than size [incorrect preposition]
beyojnd -> beyond [typographical error]
πŸ’¬ naiyoma commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2109136833)
nit: Snake case is preferred for variables. https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c
πŸ’¬ Crypt-iQ commented on issue "compact block fingerprinting":
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-2912452626)
> Dropping seemingly-unsolicited compact blocks seems fine, we already do that for the parallel portion, this should be a change just to the first one?

Yup. You wrote the code so I'm sure you know, but for viewers at home, just the first "slot" would need to be accounted for as the other two slots are [guaranteed to be HB.](https://github.com/bitcoin/bitcoin/blob/87860143be792d219aac7f4a04e79d00016df627/src/net_processing.cpp#L4506)

> So I think the CB probing, in fact, is an additional inform
...
πŸ’¬ instagibbs commented on issue "compact block fingerprinting":
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-2912461827)
> I don't think this is true anymore.

It's ostensibly forwarded once PoW/merkle checks pass, but not promising utxo/script/etc checks.