💬 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.
(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?
(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 :)
(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
(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.
(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
(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
(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!
(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.
(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.
(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
(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
(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.
(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.
(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
(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.
(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.
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310501512)
Done.
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310501512)
Done.
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310502672)
Make sense, will leave it for a possible follow-up.
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310502672)
Make sense, will leave it for a possible follow-up.
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310503817)
I think so, I can change it if I have to touch it again or in a follow-up.
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310503817)
I think so, I can change it if I have to touch it again or in a follow-up.
💬 0xB10C commented on issue "signet: disk-space-DoS due to low mining difficulty":
(https://github.com/bitcoin/bitcoin/issues/33266#issuecomment-3237765557)
> Would such blocks mined by anyone even be considered valid on signet since they would not contain any expected valid signature?
Yes. Just the nonce in the block header changes. The signet commitment commits to the header version, prevhash, merkleroot, and timestamp. It does not commit (and can not commit) to the nonce. The underlying block does not change. The commitment remains valid.
(https://github.com/bitcoin/bitcoin/issues/33266#issuecomment-3237765557)
> Would such blocks mined by anyone even be considered valid on signet since they would not contain any expected valid signature?
Yes. Just the nonce in the block header changes. The signet commitment commits to the header version, prevhash, merkleroot, and timestamp. It does not commit (and can not commit) to the nonce. The underlying block does not change. The commitment remains valid.