Bitcoin Core Github
43 subscribers
123K links
Download Telegram
🚀 achow101 merged a pull request: "util: add BitSet"
(https://github.com/bitcoin/bitcoin/pull/30160)
💬 Eunovo commented on pull request "Tr partial descriptors":
(https://github.com/bitcoin/bitcoin/pull/30243#issuecomment-2161639208)
@ajtowns Thanks for the review. I added some examples to `wallet_miniscript.py` and updated `descriptors.md`
💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#issuecomment-2161757848)
Rebased.
💬 sipa commented on pull request "feefrac: 128-bit multiply support in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29758#issuecomment-2161872276)
@hebasto Thanks. I was assuming this would be an obvious improvement, but if it isn't, it'll need some more investigation into what this is all compiled to. That's not something I'm interested in doing for a half-supported architecture.
sipa closed a pull request: "feefrac: 128-bit multiply support in MSVC"
(https://github.com/bitcoin/bitcoin/pull/29758)
💬 rocholojavinar commented on issue "Change estimate_mode default to "ECONOMICAL" in these RPC calls":
(https://github.com/bitcoin/bitcoin/issues/30009#issuecomment-2161991913)
![](https://img.youtube.com/vi/skZcNHFExzo/maxresdefault.jpg)
💬 alfonsoromanz commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1635899433)
I just pushed my recent changes for this PR. This is the approach I decided to move forward with:

1. **Scenario Choice**: Between these two scenarios mentioned by @fjahr: "Particularly 'but has less work' could mean A) less work than the tip of the chain that includes the snapshot or B) less work than the snapshot block itself," I chose B) for this PR: less work than the snapshot block itself. I am happy to add tests for the other scenario in a separate PR.
2. **Added Fix**: I incorporated t
...
💬 alfonsoromanz commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1635899839)
Fixed
💬 alfonsoromanz commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1635900277)
Fixed
💬 vasild commented on pull request "netbase: extend CreateSock() to support creating arbitrary sockets":
(https://github.com/bitcoin/bitcoin/pull/30202#discussion_r1635902059)
Done
💬 vasild commented on pull request "netbase: extend CreateSock() to support creating arbitrary sockets":
(https://github.com/bitcoin/bitcoin/pull/30202#issuecomment-2162239639)
`77e34ded54...5f549c35d9`: fix typo in the comment: `s/socke/socket/`
💬 vasild commented on pull request "netbase: extend CreateSock() to support creating arbitrary sockets":
(https://github.com/bitcoin/bitcoin/pull/30202#discussion_r1635907777)
Oh, it is usually either `s`, `sock` or `socket`. `socke` is something new, that is extra genuine ;-)

Fixed, invalidating 3 ACKs but should be trivial to re-review.

Thanks!
💬 vasild commented on pull request "fuzz: add I2P harness":
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1635926463)
Alright, I did not realize this.
(can resolve this)
👍 theStack approved a pull request: "netbase: extend CreateSock() to support creating arbitrary sockets"
(https://github.com/bitcoin/bitcoin/pull/30202#pullrequestreview-2112137538)
re-ACK 5f549c35d9a75c7019fe8a96963b988df5498eed 🧦
💬 alfonsoromanz commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#issuecomment-2162284182)
> > This PR adds tests to cover two scenarios of loading a snapshot when the current chain tip is:
> >
> > * Not an ancestor of the snapshot block but has less work
> > * Not an ancestor or a descendant of the snapshot block and has more work
> >
> > In the second scenario, the snapshot block does not belong to the most-work chain anymore so I believe it covers this scenario too: `TODO: Valid snapshot file and snapshot block, but the block is not on the most-work chain`. Therefore I delet
...
💬 S3RK commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1635953882)
nit: I think the visitor pattern in `GetSerializeSizeForRecipient` and `IsDust` in this case makes the code less clear, but I could be missing something
💬 S3RK commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#issuecomment-2162300430)
Code Review ACK f004b1625bcb9b833f36af47b53675e3dd77135b

The PR doesn't modify the existing behavior, but prepares the code for Silent Payments (see #28201)
The code pushed down in the function doesn't affect the logic, because `txnew.vout` is not used between the old and the new location.
💬 vasild commented on pull request "fuzz: add I2P harness":
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1635958429)
The frozen time will defeat `FUZZED_SOCKET_FAKE_LATENCY`:

https://github.com/bitcoin/bitcoin/blob/91e0beede2859dea987ba6db746e95dca0ceb024/src/test/fuzz/util/net.cpp#L229-L231

`RecvUntilTerminator()` and `SendComplete()` will never timeout. Is this acceptable trade off in order to have the fuzz test "more stable"? If it is "unstable" without freezing the time, does that mean that some of those methods actually timed out, resulting in a wider coverage? In this aspect, it seems that stable =
...
💬 S3RK commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1635958466)
p.s. I don't see how the callable will be applied to more than one destination, so in my mind it's just easier to inline the callable and drop the `std::visit`
👍 alfonsoromanz approved a pull request: "assumeutxo: Check snapshot base block is not in invalid chain"
(https://github.com/bitcoin/bitcoin/pull/30267#pullrequestreview-2112255189)
tACK cbc604f6a7739bf22a5d811b6366453860d59f4c

This PR successfully builds and passes the test when running `test/functional/feature_assumeutxo.py`.

Additionally, I tested this in the context of my PR ([#29996](https://github.com/bitcoin/bitcoin/pull/29996)) back when the code looked something like this:

```
def test_snapshot_in_a_divergent_chain(self, dump_output_path):
node = self.nodes[2]
# Rollback the chain down to START_HEIGHT
block_hash = node.getbl
...