Bitcoin Core Github
44 subscribers
121K links
Download Telegram
🤔 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.
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(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.
💬 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.
💬 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.
💬 achow101 commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3237941668)
> Is this context info useful? Line numbers will change whenever some previous function in the file gets a non-trivial edit... Would it be better to add `-locations none` to the LCONVERT invocation in translate.cmake?

I don't think line numbers is the problem. The actual source location of the string can change and the ids in the `.xlf` won't change. It's the sort order in the `.ts` file that `lupdate` generates that seems to cause the id change. It's unclear to me why the sort order would ch
...
👍 l0rinc approved a pull request: "test: Fixup fill_mempool docstring"
(https://github.com/bitcoin/bitcoin/pull/33269#pullrequestreview-3169923720)
ACK fa3f682032a3292604f363a5ee4557937f3d8950
💬 polespinasa commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2311028761)
Done
💬 achow101 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_r2311048525)
Updated the commit message.
💬 achow101 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_r2311048812)
Clarified the test name