Bitcoin Core Github
43 subscribers
122K links
Download Telegram
📝 fjahr opened a pull request: "index: Fix coinstats overflow and introduce index versioning"
(https://github.com/bitcoin/bitcoin/pull/30469)
Closes https://github.com/bitcoin/bitcoin/issues/26362

This continues the work that was started with #26426. It fixes the overflow issue by switching most of the values tracked in the index from historical totals to values per block. The only effect of this is that it requires iterating over all the entries to get to these historical values, however nothing changes for the capabilities of the RPCs we have today because the switched values were always report per block and the totals were used
...
💬 fjahr commented on pull request "WIP: Fix coinstatsindex overflow issue":
(https://github.com/bitcoin/bitcoin/pull/26426#issuecomment-2233006443)
#30469 continues the work here
💬 fjahr commented on issue "signed overflow in coinstats index":
(https://github.com/bitcoin/bitcoin/issues/26362#issuecomment-2233007278)
#30469 is a new try at fixing this
💬 fjahr commented on pull request "index: Check all necessary block data is available before starting to sync":
(https://github.com/bitcoin/bitcoin/pull/29770#issuecomment-2233022252)
just rebased
📝 Igor258 opened a pull request: "btc"
(https://github.com/bitcoin/bitcoin/pull/30470)
<!--
*** 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
...
fanquake closed a pull request: "btc"
(https://github.com/bitcoin/bitcoin/pull/30470)
📝 fanquake locked a pull request: "btc"
(https://github.com/bitcoin/bitcoin/pull/30470)
<!--
*** 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
...
👍 stickies-v approved a pull request: "qa: Functional test improvements"
(https://github.com/bitcoin/bitcoin/pull/30463#pullrequestreview-2182628847)
ACK a8e3af1a82dd584a1cc3ffbe587e66889f72e3c7

I'm able to run individual tests from an out-of-source build with the instructions provided. I tried running a bunch of other tests than `wallet_disable` too, including all the ones that reference `__file__` and those seem to work fine too.
💬 vasild commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1680885121)
I admit I am somewhat confused.

Do we assume that for IPv6, address translation is never done (e.g. https://www.rfc-editor.org/rfc/rfc6296) and that for IPv6 we only need to pinhole the firewall?

> For IPv6 we know what internal address we want, and what external address we want

You mean that both internal and external are the same and we get that from `GetLocalAddresses()`?
👍 tdb3 approved a pull request: "doc: getaddressinfo[isscript] is optional"
(https://github.com/bitcoin/bitcoin/pull/30457#pullrequestreview-2182648657)
ACK fa6390df205513319f28e35e3e17c40ecaa7d731
💬 maflcko commented on pull request "qa: Functional test improvements":
(https://github.com/bitcoin/bitcoin/pull/30463#discussion_r1680899761)
Maybe, but it would be good to clarify what of the previous conversation is relevant.

Are you saying that this is needed for Windows and can not be tested on Unix?
💬 maflcko commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r1680902695)
:eye:
💬 maflcko commented on pull request "test: Non-Shy version sender":
(https://github.com/bitcoin/bitcoin/pull/30453#discussion_r1680905532)
Thanks, taken!
💬 maflcko commented on pull request "test: bump mocktime only after node has received and sent bytes":
(https://github.com/bitcoin/bitcoin/pull/30468#discussion_r1680907742)
This will be racy, no?

The peer timeout is `3`, which is also the bump, so the peer may be disconnected at any time, leading to a test failure if it is disconnected at the wrong time.

Something like this should fail the test, no?

```suggestion
node0.bumpmocktime(3); time.sleep(1);
```
👋 maflcko's pull request is ready for review: "rest: Reject negative outpoint index early in getutxos parsing"
(https://github.com/bitcoin/bitcoin/pull/30444)
💬 maflcko commented on pull request "rest: Reject negative outpoint index early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30444#issuecomment-2233122996)
Pushed a commit to fix a harmless `unsigned-integer-overflow` warning.

@hodlinator Happy to review your suggestions to other functions in a separate pull request, if you create one. But I'll try to keep this one focussed for now.
⚠️ craigraw opened an issue: "Feature Request: Broadcast Pool"
(https://github.com/bitcoin/bitcoin/issues/30471)
### Please describe the feature you'd like to see added.

This is a feature request for a broadcast pool. A broadcast pool is a cache local to the node which contains transactions which have been initially broadcast from that node, either from a local wallet or via an RPC like `sendrawtransaction`. Broadcast transactions are retained in this cache until they are included in a block, or expire after the configured `mempoolexpiry` period. The transactions in the broadcast pool are included in view
...
💬 hebasto commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2233129404)
> > From [zeromq/libzmq#4667 (comment)](https://github.com/zeromq/libzmq/pull/4667#issue-2197755398):
>
> Can you elaborate / suggest something concrete?

Please see https://github.com/zeromq/libzmq/pull/4706.
👍 hodlinator approved a pull request: "rest: Reject negative outpoint index early in getutxos parsing"
(https://github.com/bitcoin/bitcoin/pull/30444#pullrequestreview-2182729770)
ut-ACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1

Nice that the added tests managed to trigger the sanitizer! :+1:
💬 vasild commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1680951137)
Would all-zeros work as well for IPv6 / pinholing? If yes, then this can probably be simplified to always pass all-zeros.