🤔 vasild reviewed a pull request: "doc: i2p: improve `-i2pacceptincoming` mention"
(https://github.com/bitcoin/bitcoin/pull/29813#pullrequestreview-2052781798)
ACK 2179e2c3209a41c1419f1f5ed6270a0dad68b50d
(https://github.com/bitcoin/bitcoin/pull/29813#pullrequestreview-2052781798)
ACK 2179e2c3209a41c1419f1f5ed6270a0dad68b50d
👍 TheCharlatan approved a pull request: "rpc: move UniValue in blockToJSON"
(https://github.com/bitcoin/bitcoin/pull/30094#pullrequestreview-2052782996)
ACK b77bad309e92f176f340598eec056eb7bff86f5f
(https://github.com/bitcoin/bitcoin/pull/30094#pullrequestreview-2052782996)
ACK b77bad309e92f176f340598eec056eb7bff86f5f
💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598526584)
Ok I've added a commit to fix this and another log to say "transaction(s)". I took the opportunity to add a few nice things to the logs as well.
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598526584)
Ok I've added a commit to fix this and another log to say "transaction(s)". I took the opportunity to add a few nice things to the logs as well.
💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598461970)
added comment
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598461970)
added comment
💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598466023)
added
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598466023)
added
💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598461107)
fixed
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598461107)
fixed
💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598463139)
done
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598463139)
done
💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598496074)
I would say so. We do all `ProcessOrphanTx` before processing the ping; the child will be added to workset when the parent is accepted through `ProcessOrphanTx`
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598496074)
I would say so. We do all `ProcessOrphanTx` before processing the ping; the child will be added to workset when the parent is accepted through `ProcessOrphanTx`
💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598497584)
changed to be more perscriptive
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598497584)
changed to be more perscriptive
💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598462421)
done
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598462421)
done
💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598499228)
done
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598499228)
done
💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598465418)
elaborated
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598465418)
elaborated
💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#issuecomment-2107656257)
Thanks @instagibbs @theStack @sr-gi! Addressed the comments and added a few minor log improvements.
(https://github.com/bitcoin/bitcoin/pull/30000#issuecomment-2107656257)
Thanks @instagibbs @theStack @sr-gi! Addressed the comments and added a few minor log improvements.
💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598532513)
this was (mostly) taken
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598532513)
this was (mostly) taken
💬 sr-gi commented on pull request "net: Favor peers from addrman over fetching seednodes":
(https://github.com/bitcoin/bitcoin/pull/29605#issuecomment-2107677632)
@cbergqvist I took your suggestion from https://github.com/bitcoin/bitcoin/pull/29605#discussion_r1586847044
(https://github.com/bitcoin/bitcoin/pull/29605#issuecomment-2107677632)
@cbergqvist I took your suggestion from https://github.com/bitcoin/bitcoin/pull/29605#discussion_r1586847044
💬 Eunovo commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#issuecomment-2107736166)
> Perhaps another approach could be a class for `RemovalReason`, which contains the Enum as a field and then uses a std::variant to hold the data needed by the wallet, if any. Would be more extensible, and then the to string method could be defined once on the class. Special logic per reason can also be handled all in the same place by switching on the Enum value and taking appropriate action per reason.
Thank you @josibake. Will this approach still require the definition of structs to hold
...
(https://github.com/bitcoin/bitcoin/pull/29680#issuecomment-2107736166)
> Perhaps another approach could be a class for `RemovalReason`, which contains the Enum as a field and then uses a std::variant to hold the data needed by the wallet, if any. Would be more extensible, and then the to string method could be defined once on the class. Special logic per reason can also be handled all in the same place by switching on the Enum value and taking appropriate action per reason.
Thank you @josibake. Will this approach still require the definition of structs to hold
...
🤔 ryanofsky reviewed a pull request: "kernel: Streamline util library"
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2052882564)
Updated 6ea5ee5a268cfbb9cd234e0f1381c8a4785ff04c -> 43532d9c9470ad00ecb991859f7f0c6a025fa72e ([`pr/rmutil.14`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.14) -> [`pr/rmutil.15`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.15), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/rmutil.14..pr/rmutil.15)) fixing silent conflict with #29845, fixing shellcheck lint errors, and moving check-deps.sh commit first as suggested so it is easier to see effects of later commits.
...
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2052882564)
Updated 6ea5ee5a268cfbb9cd234e0f1381c8a4785ff04c -> 43532d9c9470ad00ecb991859f7f0c6a025fa72e ([`pr/rmutil.14`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.14) -> [`pr/rmutil.15`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.15), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/rmutil.14..pr/rmutil.15)) fixing silent conflict with #29845, fixing shellcheck lint errors, and moving check-deps.sh commit first as suggested so it is easier to see effects of later commits.
...
💬 glozow commented on pull request "rpc, bugfix: Enforce maximum value for setmocktime":
(https://github.com/bitcoin/bitcoin/pull/29869#issuecomment-2107746080)
Backport for 26.x is in #29899
(https://github.com/bitcoin/bitcoin/pull/29869#issuecomment-2107746080)
Backport for 26.x is in #29899
💬 paplorinc commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1598594927)
What if we just extract it to code like it was done with the [rest of the sage files](https://github.com/search?q=repo%3Abitcoin%2Fbitcoin%20.sage&type=code), and reference the file instead of the code (I don't like dead code in comments), e.g. https://github.com/bitcoin/bitcoin/blob/master/src/secp256k1/src/ecdsa_impl.h#L19 ?
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1598594927)
What if we just extract it to code like it was done with the [rest of the sage files](https://github.com/search?q=repo%3Abitcoin%2Fbitcoin%20.sage&type=code), and reference the file instead of the code (I don't like dead code in comments), e.g. https://github.com/bitcoin/bitcoin/blob/master/src/secp256k1/src/ecdsa_impl.h#L19 ?
💬 josibake commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#issuecomment-2107797730)
> Will this approach still require the definition of structs to hold each data for each reason?
I don't think so: only the Conflict and Replace reasons need extra data, and from what I understand it can only be one reason at any given time. Given that, you could have a field on the new class for removal data which is a std::variant of the possible data types. For example, `std::variant<CTransactionRef, BlockData>`, where block data holds the block hash and block number.
(https://github.com/bitcoin/bitcoin/pull/29680#issuecomment-2107797730)
> Will this approach still require the definition of structs to hold each data for each reason?
I don't think so: only the Conflict and Replace reasons need extra data, and from what I understand it can only be one reason at any given time. Given that, you could have a field on the new class for removal data which is a std::variant of the possible data types. For example, `std::variant<CTransactionRef, BlockData>`, where block data holds the block hash and block number.