💬 itornaza commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1780104846)
Do you think defining this as a const function `std::vector<OrphanTxBase> GetOrphanTransactions() const;` makes sense? I do not think it changes any members of the `TxOrphanage`.
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1780104846)
Do you think defining this as a const function `std::vector<OrphanTxBase> GetOrphanTransactions() const;` makes sense? I do not think it changes any members of the `TxOrphanage`.
📝 l0rinc opened a pull request: "test: streamline CheckFormatSpecifiers testability"
(https://github.com/bitcoin/bitcoin/pull/30999)
Split out of https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779584565
The current string formatter couldn't validate every string format template that we were using.
Extended it with dynamic widths, fixed a number parsing bug that could go over the string's content and added a `%n` validation.
The tests are all runtime checked now - the rest of the production code is still compile-time checked.
This simplifies debugging (e.g. debugging why invalid values are parsed differentl
...
(https://github.com/bitcoin/bitcoin/pull/30999)
Split out of https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779584565
The current string formatter couldn't validate every string format template that we were using.
Extended it with dynamic widths, fixed a number parsing bug that could go over the string's content and added a `%n` validation.
The tests are all runtime checked now - the rest of the production code is still compile-time checked.
This simplifies debugging (e.g. debugging why invalid values are parsed differentl
...
💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780117724)
Extracted it to https://github.com/bitcoin/bitcoin/pull/30999/files#diff-71badc1cc71ba46244f7841a088251bb294265f4fe9e662c0ad6b15be540eee4R45
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780117724)
Extracted it to https://github.com/bitcoin/bitcoin/pull/30999/files#diff-71badc1cc71ba46244f7841a088251bb294265f4fe9e662c0ad6b15be540eee4R45
💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1780149394)
Thanks! Will include this in next push.
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1780149394)
Thanks! Will include this in next push.
👍 l0rinc approved a pull request: "test: Remove dead code from interface_zmq test"
(https://github.com/bitcoin/bitcoin/pull/30942#pullrequestreview-2335985809)
ACK eb20d8263b0da828624261eb9df0f9cc1f4f9f96
(https://github.com/bitcoin/bitcoin/pull/30942#pullrequestreview-2335985809)
ACK eb20d8263b0da828624261eb9df0f9cc1f4f9f96
💬 fjahr commented on pull request "test: Remove dead code from interface_zmq test":
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157551)
`get_raw_seq` is always 6 and `num_txs` is always 5, so we can write it like this. Not sure if this is an improvement but it doesn't hurt so I have changed it.
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157551)
`get_raw_seq` is always 6 and `num_txs` is always 5, so we can write it like this. Not sure if this is an improvement but it doesn't hurt so I have changed it.
💬 l0rinc commented on pull request "test: Remove dead code from interface_zmq test":
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157721)
should be `num_txs + `
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157721)
should be `num_txs + `
💬 fjahr commented on pull request "test: Remove dead code from interface_zmq test":
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157728)
Added as belt and suspender
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157728)
Added as belt and suspender
💬 fjahr commented on pull request "test: Remove dead code from interface_zmq test":
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157944)
fixed
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157944)
fixed
💬 l0rinc commented on pull request "test: Remove dead code from interface_zmq test":
(https://github.com/bitcoin/bitcoin/pull/30942#issuecomment-2381580319)
ACK c4dc81f9c6980964f63b9ad5166cd4cfaa86f3e6
(https://github.com/bitcoin/bitcoin/pull/30942#issuecomment-2381580319)
ACK c4dc81f9c6980964f63b9ad5166cd4cfaa86f3e6
👍 tdb3 approved a pull request: "test: Remove dead code from interface_zmq test"
(https://github.com/bitcoin/bitcoin/pull/30942#pullrequestreview-2335986876)
CR re ACK c4dc81f9c6980964f63b9ad5166cd4cfaa86f3e6
(https://github.com/bitcoin/bitcoin/pull/30942#pullrequestreview-2335986876)
CR re ACK c4dc81f9c6980964f63b9ad5166cd4cfaa86f3e6
💬 beage666 commented on pull request "test: Remove dead code from interface_zmq test":
(https://github.com/bitcoin/bitcoin/pull/30942#issuecomment-2381599058)
Ok
(https://github.com/bitcoin/bitcoin/pull/30942#issuecomment-2381599058)
Ok
👋 l0rinc's pull request is ready for review: "Cover remaining tinyformat usages in CheckFormatSpecifiers"
(https://github.com/bitcoin/bitcoin/pull/30999)
(https://github.com/bitcoin/bitcoin/pull/30999)
📝 furszy opened a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000)
Expands the benchmark framework with the existing `-testdatadir` arg,
enabling the ability to change the benchmark data directory.
This is useful for running benchmarks on different storage devices, and
not just under the OS `/tmp/` directory.
A good use case is #28574, where we are benchmarking the wallet
migration process on an HDD.
(https://github.com/bitcoin/bitcoin/pull/31000)
Expands the benchmark framework with the existing `-testdatadir` arg,
enabling the ability to change the benchmark data directory.
This is useful for running benchmarks on different storage devices, and
not just under the OS `/tmp/` directory.
A good use case is #28574, where we are benchmarking the wallet
migration process on an HDD.
💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1780172239)
Yeah, looking at it again, this doesn't make sense because the test may fail if someone actually scales the wait time down. I changed it back and will leave the mocktime approach for a follow-up.
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1780172239)
Yeah, looking at it again, this doesn't make sense because the test may fail if someone actually scales the wait time down. I changed it back and will leave the mocktime approach for a follow-up.
💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1780172255)
Changed to use `**kwargs` in both `ensure_for` impls
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1780172255)
Changed to use `**kwargs` in both `ensure_for` impls
💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1780172263)
I agree and couldn't find any good use cases for it. It can be re-added when there is a use case for it so I have removed it for now and simplified the code.
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1780172263)
I agree and couldn't find any good use cases for it. It can be re-added when there is a use case for it so I have removed it for now and simplified the code.
💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2381605700)
Addressed feedback from @maflcko , thanks!
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2381605700)
Addressed feedback from @maflcko , thanks!
👍 tdb3 approved a pull request: "test: Introduce ensure_for helper"
(https://github.com/bitcoin/bitcoin/pull/30893#pullrequestreview-2336023567)
re ACK 352b8209aa5327f7d369e2acc4d87f9767389a6b
Also sanity checked testing timing (https://github.com/bitcoin/bitcoin/pull/30893?notification_referrer_id=NT_kwDOBljilbUxMjM4MjY5Nzc4OToxMDY0ODg0Njk#pullrequestreview-2316167311)
(https://github.com/bitcoin/bitcoin/pull/30893#pullrequestreview-2336023567)
re ACK 352b8209aa5327f7d369e2acc4d87f9767389a6b
Also sanity checked testing timing (https://github.com/bitcoin/bitcoin/pull/30893?notification_referrer_id=NT_kwDOBljilbUxMjM4MjY5Nzc4OToxMDY0ODg0Njk#pullrequestreview-2316167311)
🤔 tdb3 reviewed a pull request: "Split CConnman"
(https://github.com/bitcoin/bitcoin/pull/30988#pullrequestreview-2336026243)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30988#pullrequestreview-2336026243)
Concept ACK