Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 hodlinator commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2309887787)
nit: I think this part is repetitive and somewhat arbitrary, currently the PR ensures sigop-adjusted and non-sigop-adjusted sizes are available for all mentioned RPCs. If I was using RPCs for each transaction for this calculation I would just pick the fastest one. Maybe I'm overlooking something though.
```suggestion
```
💬 hodlinator commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2309870327)
nit: The adjusted beginning is nicer, but would rather bring back the end from 1523e8d6c981efff1394bd1669fbbbea5ff7ac97:
> Additionally, `getrawtransaction` now includes
+a `sigopsize` field and continues to report the unadjusted BIP-141 virtual size in its `vsize` field.
💬 jsarenik commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3236825770)
After it gets merged into `master` branch, I would recommend backporting it to current stable release branch which already contains the ephemeral outputs support ([29.x](https://github.com/bitcoin/bitcoin/blob/7cc9a087069bfcdb79a08ce77eb3a60adf9483af/doc/release-notes/release-notes-29.0.md?plain=1#L64}).
💬 jsarenik commented on issue "signet: disk-space-DoS due to low mining difficulty":
(https://github.com/bitcoin/bitcoin/issues/33266#issuecomment-3236886747)
Would such blocks mined by anyone even be considered valid on signet since they would not contain any expected valid signature?
💬 Crypt-iQ commented on pull request "build: set ENABLE_IPC to OFF when fuzzing":
(https://github.com/bitcoin/bitcoin/pull/33235#issuecomment-3236937041)
tACK af4156ab75565acc5a71b68e70da5e2cf407df80

I ran into build failures while trying to measure fuzz coverage.
💬 instagibbs commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2309954963)
> which is necessary for marking anchor outputs as spent.

To be pedantic, it's not about PayToAnchor(P2A), which can be any value, but any 0-value outputs, even if they weren't ephemeral (say a miner mined it to dust your wallet).

There are other legitimate 0-value output use-cases such as connector outputs, so let's just call it 0-value dust?
💬 instagibbs commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2310110919)
this subtest is covering 0-value outputs, not anchors per se. I'd rather keep the tests logically separated to not confused the concepts more than they already are :)
💬 instagibbs commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3236975180)
@jsarenik technically this issue could happen if miners mine 0-value outputs of any kind, but given that it's now relay-possible, seems like a good idea to backport
💬 ajtowns commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#issuecomment-3236980424)
Rebased over #33253 but not optimised to iterate over template txs efficiently.
🤔 stickies-v reviewed a pull request: "rpc: refactor: use string_view in Arg/MaybeArg"
(https://github.com/bitcoin/bitcoin/pull/32983#pullrequestreview-3168699729)
Force-pushed to address review comments from @maflcko:
- fixed docstring
- removed usage of std::string_view::data() to avoid null termination issues in the future
- added one more usage of Arg<string_view> helper
💬 stickies-v commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#discussion_r2310136874)
DELETE
💬 stickies-v commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#discussion_r2310142429)
Done!
💬 stickies-v commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#discussion_r2310142118)
I agree. Even though there are no issues now, it's safer to avoid using `.data()` altogether. I've adopted your suggestion. I'll probably open a separate PR later today to change the necessary `net` code to allow us to `string_view` here. It's a relatively straightforward change, but I don't want to scope creep this PR more.
💬 stickies-v commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#discussion_r2310144112)
Done. Kept the `get_str()` because we need a `std::string`, but your suggestion is clean.
💬 bigshiny90 commented on pull request "test: Add functional tests for blockreconstructionextratxn and extra pool (compactblocks)":
(https://github.com/bitcoin/bitcoin/pull/33023#issuecomment-3237047651)
investigating why tests fail on github CI... but run fine on a local build
🤔 janb84 reviewed a pull request: "test: p2p block malleability"
(https://github.com/bitcoin/bitcoin/pull/33172#pullrequestreview-3168806620)
re ACK d0e1bbad016cc4949094daea2934712f92dfeecd

Changes since last ACK;

- small NIT suggested changes
💬 bigshiny90 commented on pull request "test: Add functional tests for blockreconstructionextratxn and extra pool (compactblocks)":
(https://github.com/bitcoin/bitcoin/pull/33023#issuecomment-3237148592)
needed to rebase to current master and fix tests for the new lower fee-rate policy.
📝 fanquake opened a pull request: "[29.x] rc3 or final"
(https://github.com/bitcoin/bitcoin/pull/33271)
Backports:
* #33236

Might include #33268.
Since `rc2` #33212 was also backported in #33251.
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310498917)
Done
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310501279)
I moved `MockMempoolMinFee` to mempool utils to use it here and in the unit tests. I had to add one more parameter which is the mempool to be mocked.