Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1424191480)
let's check the error messages here and elsewhere to prevent test regressions
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1424237443)
Now that V3 checks are split into 4 different areas, I think we need to brainstorm a better way of naming this function, and describing exactly what it's *not* covering. Encapsulating the final check with something very similar will probably help with my pattern matching, ala `CheckV3Inheritence`?
💬 mohamedawnallah commented on issue "test: Add TestNode wait_until helper":
(https://github.com/bitcoin/bitcoin/issues/29029#issuecomment-1852450452)
Hey, @nikodemas Are you still working on this issue? If not I’d like to pick it up. Thanks
💬 mohamedawnallah commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-1852489784)
Hey, @jrakibi Are you still working on this issue? If not I’d like to pick it up. Thanks
💬 fanquake commented on pull request "[26.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/29011#issuecomment-1852519450)
Note that any tidy job failure here is currently related to https://github.com/bitcoin/bitcoin/pull/28992#issuecomment-1852101216 (and should be fixed in future).
🤔 stickies-v reviewed a pull request: "doc: Add multiprocess design doc"
(https://github.com/bitcoin/bitcoin/pull/28978#pullrequestreview-1777912181)
Approach ACK. I'm still working on my understanding of multiprocess but this document was very helpful already in its current state, no major suggestions at this point.
💬 stickies-v commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1424248273)
typo nit
```suggestion
The choice to use an RPC framework at all instead of a custom protocol was necessitated by the size of Bitcoin Core internal interfaces which consist of around 150 methods that pass complex data structures and are called in complicated ways (in parallel, from callbacks that can be nested and stored). Writing a custom protocol to wrap these complicated interfaces would be too much work, akin to writing a new RPC framework.
```
💬 stickies-v commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1424249827)
nit: rogue backtick
```suggestion
- The call to `getBlockHash` is translated into a Cap’n Proto RPC call. This translation is handled by code automatically generated by the `mpgen` tool, based on the [`chain.capnp`](../../src/ipc/capnp/chain.capnp) file in [`src/ipc/capnp/`](../../src/ipc/capnp/).
```
💬 stickies-v commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1424253642)
Steps 2 and 3 highlight what's handled by generated code, for consistency that might be helpful in steps 4&5 too?
💬 mohamedawnallah commented on issue "Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/issues/27494#issuecomment-1852524854)
Hey, @theartpiece, Are you still working on this issue? If not, I’d like to pick it up. Thanks
📝 dergoegge opened a pull request: "fuzz: Improve fuzzing stability for minisketch harness"
(https://github.com/bitcoin/bitcoin/pull/29064)
The `minisketch` harness has low stability due to:
* Rng internal to minisketch
* Benchmarkning for the best minisketch impl

Fix this by seeding the rng and fixing the impl to 0.

Also see #29018.
💬 eragmus commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1852552451)
> When the mempool is clogged with this kind of TXs, users trying to use Bitcoin as money are priced out in droves, because they can't pay 98% of their UTXOs in fees, so they can only compete by moving orders of magnitude more value than inscriptions. Therefore when inscriptions are setting the floor at $10, they're not just pricing out people trying to transact $10 onchain, but also $20, $50, $100.. maybe up to $500.

I think the linked X thread's analysis and your analysis are both wrong. What
...
💬 Sjors commented on pull request "Stratum v2 Template Provider (take 2)":
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1852558078)
Ran clang-tidy locally to (hopefully) fix its remaining complaints. Included @Fi3's https://github.com/Sjors/bitcoin/pull/25 (plus additional header and test change) which drops the `CIPHER_CONFIRMATION` state, as per https://github.com/stratum-mining/sv2-spec/issues/60.
👍 maflcko approved a pull request: "fuzz: Improve fuzzing stability for minisketch harness"
(https://github.com/bitcoin/bitcoin/pull/29064#pullrequestreview-1778170510)
lgtm
💬 maflcko commented on pull request "fuzz: Improve fuzzing stability for minisketch harness":
(https://github.com/bitcoin/bitcoin/pull/29064#discussion_r1424406089)
Why hardcode this to `0`? Wouldn't it be better to let the fuzz engine pick one of `MaxImplementation()`?
💬 maflcko commented on pull request "fuzz: Improve fuzzing stability for minisketch harness":
(https://github.com/bitcoin/bitcoin/pull/29064#discussion_r1424406965)
Is this needed? Both should have been seeded already, no?
💬 maflcko commented on pull request "fuzz: Improve fuzzing stability for minisketch harness":
(https://github.com/bitcoin/bitcoin/pull/29064#discussion_r1424406512)
When touching this, maybe move the Assert into the helper?
💬 1ma commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1852569550)
> > When the mempool is clogged with this kind of TXs, users trying to use Bitcoin as money are priced out in droves, because they can't pay 98% of their UTXOs in fees, so they can only compete by moving orders of magnitude more value than inscriptions. Therefore when inscriptions are setting the floor at $10, they're not just pricing out people trying to transact $10 onchain, but also $20, $50, $100.. maybe up to $500.
>
> I think the linked X thread's analysis and your analysis are both wro
...
🤔 mzumsande reviewed a pull request: "p2p: Fill reconciliation sets (Erlay)"
(https://github.com/bitcoin/bitcoin/pull/28765#pullrequestreview-1778050698)
Code Review ACK f56ec8a3b086b0f2f8a0fbde861447e40ffbc3d9

One thing I'm unsure about is how the way we call `ShouldFanoutTo()` multiple times for each transaction would might affect performance.
If we have 120 reconciling peers, and get a dump of `MAX_SET_SIZE=3000` transactions, I think we'd call this function `360000` times within a short timeframe. I wonder how long that would take, maybe we could add a benchmark for this? (could be done a follow-up)
💬 mzumsande commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1424331514)
I think that `deterministic_randomizer` could be passed by reference