💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2087315636)
Rebased
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2087315636)
Rebased
💬 achow101 commented on pull request "seeds: Pull additional nodes from my seeder and update fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/30008#discussion_r1585525305)
They were updated in #29561, and the addresses were pulled from my node's addrman. Some sorting happened somewhere, and because `makeseeds.py` doesn't shuffle (this PR adds a commit that does that), when it applied the max node count, it ended up with the tail end of that list.
(https://github.com/bitcoin/bitcoin/pull/30008#discussion_r1585525305)
They were updated in #29561, and the addresses were pulled from my node's addrman. Some sorting happened somewhere, and because `makeseeds.py` doesn't shuffle (this PR adds a commit that does that), when it applied the max node count, it ended up with the tail end of that list.
💬 naiyoma commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#issuecomment-2087518282)
Concept ACK covers more tests scenarios for AssumeUTXO
(https://github.com/bitcoin/bitcoin/pull/29996#issuecomment-2087518282)
Concept ACK covers more tests scenarios for AssumeUTXO
🤔 achow101 reviewed a pull request: "p2p: opportunistically accept 1-parent-1-child packages"
(https://github.com/bitcoin/bitcoin/pull/28970#pullrequestreview-2032571316)
ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
(https://github.com/bitcoin/bitcoin/pull/28970#pullrequestreview-2032571316)
ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
💬 achow101 commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1585554972)
In 092c978a42e8f4a02291b994713505ba8aac8b28 "[txpackages] add canonical way to get hash of package"
Is there a reason to convert the wtxids into hex strings for this comparison? That seems kind of expensive, especially when `Wtxid` already has an `operator<`.
I surmise that this has to do with the "concatenated in lexicographical order (treating the wtxids as little endian encoded uint256, smallest to largest)" and BIP 331 defines the same thing, but it's not clear to me why it must be lik
...
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1585554972)
In 092c978a42e8f4a02291b994713505ba8aac8b28 "[txpackages] add canonical way to get hash of package"
Is there a reason to convert the wtxids into hex strings for this comparison? That seems kind of expensive, especially when `Wtxid` already has an `operator<`.
I surmise that this has to do with the "concatenated in lexicographical order (treating the wtxids as little endian encoded uint256, smallest to largest)" and BIP 331 defines the same thing, but it's not clear to me why it must be lik
...
💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1585616207)
made a similar point at https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571063644 fwiw (with a response attached)
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1585616207)
made a similar point at https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571063644 fwiw (with a response attached)
🚀 achow101 merged a pull request: "p2p: opportunistically accept 1-parent-1-child packages"
(https://github.com/bitcoin/bitcoin/pull/28970)
(https://github.com/bitcoin/bitcoin/pull/28970)
🤔 mzumsande reviewed a pull request: "p2p: opportunistically accept 1-parent-1-child packages"
(https://github.com/bitcoin/bitcoin/pull/28970#pullrequestreview-2032696340)
Light ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
I didn't review the functional tests in much detail, but the p2p code looks good to me.
(https://github.com/bitcoin/bitcoin/pull/28970#pullrequestreview-2032696340)
Light ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
I didn't review the functional tests in much detail, but the p2p code looks good to me.
💬 mzumsande commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1585612529)
nit: I think that 6c51e1d7d021ed6523107a6db87a865aaa8fc4c9 changes behavior such that txns which are now in `m_recent_rejects_reconsiderable` instead of `m_recent_rejects` will be ignored when calling `m_recent_rejects.contains(...)`. That is probably an undesired change of behavior - however it's just for 1 commit :man_shrugging:
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1585612529)
nit: I think that 6c51e1d7d021ed6523107a6db87a865aaa8fc4c9 changes behavior such that txns which are now in `m_recent_rejects_reconsiderable` instead of `m_recent_rejects` will be ignored when calling `m_recent_rejects.contains(...)`. That is probably an undesired change of behavior - however it's just for 1 commit :man_shrugging:
💬 mzumsande commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1585614142)
I agree, would be nice to avoid this code duplication.
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1585614142)
I agree, would be nice to avoid this code duplication.
💬 mzumsande commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1585616821)
I wonder if there are situations in which we fail on a package level and therefore return here and never call `ProcessInvalidTx()` to remove the child from the orphanage. Would that be possible, for example, with v3 transactions that violate `PackageV3Checks()`, and is there something we could do against it? (or should, maybe it's not that terrible?).
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1585616821)
I wonder if there are situations in which we fail on a package level and therefore return here and never call `ProcessInvalidTx()` to remove the child from the orphanage. Would that be possible, for example, with v3 transactions that violate `PackageV3Checks()`, and is there something we could do against it? (or should, maybe it's not that terrible?).
⚠️ BullishNode opened an issue: "Change estimate_mode default to "ECONOMICAL" in these RPC calls"
(https://github.com/bitcoin/bitcoin/issues/30009)
### Please describe the feature you'd like to see added.
The default fee estimate mode for the following RPC calls is set to "ECONOMICAL" rather than "CONSERVATIVE".
My observation running a Bitcoin non-custodial exchange and payment processor which creates a lot transactions is that Bitcoin Core on conservative mode consistently overpays transaction fees as compared to mempool's fee estimation algorithm. I have collected data on this, if anyone needs convincing, but I think this is common k
...
(https://github.com/bitcoin/bitcoin/issues/30009)
### Please describe the feature you'd like to see added.
The default fee estimate mode for the following RPC calls is set to "ECONOMICAL" rather than "CONSERVATIVE".
My observation running a Bitcoin non-custodial exchange and payment processor which creates a lot transactions is that Bitcoin Core on conservative mode consistently overpays transaction fees as compared to mempool's fee estimation algorithm. I have collected data on this, if anyone needs convincing, but I think this is common k
...
✅ achow101 closed an issue: "AddTimeData will never update nTimeOffset past 199 samples"
(https://github.com/bitcoin/bitcoin/issues/4521)
(https://github.com/bitcoin/bitcoin/issues/4521)
🚀 achow101 merged a pull request: "Simplify network-adjusted time warning logic"
(https://github.com/bitcoin/bitcoin/pull/29623)
(https://github.com/bitcoin/bitcoin/pull/29623)
💬 BullishNode commented on issue "Change estimate_mode default to "ECONOMICAL" in these RPC calls":
(https://github.com/bitcoin/bitcoin/issues/30009#issuecomment-2087655291)
Another option would be to put this in the config.
(https://github.com/bitcoin/bitcoin/issues/30009#issuecomment-2087655291)
Another option would be to put this in the config.
💬 achow101 commented on pull request "p2p: gives seednode priority over dnsseed if both are provided":
(https://github.com/bitcoin/bitcoin/pull/28016#issuecomment-2087660085)
ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d
(https://github.com/bitcoin/bitcoin/pull/28016#issuecomment-2087660085)
ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d
💬 BullishNode commented on issue "Change estimate_mode default to "ECONOMICAL" in these RPC calls":
(https://github.com/bitcoin/bitcoin/issues/30009#issuecomment-2087669682)
Another option would be to have a config for default estimate mode but in that case also the default should be "ECONOMICAL" and not "CONSERVATIVE"
(https://github.com/bitcoin/bitcoin/issues/30009#issuecomment-2087669682)
Another option would be to have a config for default estimate mode but in that case also the default should be "ECONOMICAL" and not "CONSERVATIVE"
💬 achow101 commented on pull request "doc: i2p: improve `-i2pacceptincoming` mention":
(https://github.com/bitcoin/bitcoin/pull/29813#issuecomment-2087670358)
ACK 2179e2c3209a41c1419f1f5ed6270a0dad68b50d
(https://github.com/bitcoin/bitcoin/pull/29813#issuecomment-2087670358)
ACK 2179e2c3209a41c1419f1f5ed6270a0dad68b50d
🚀 achow101 merged a pull request: "p2p: gives seednode priority over dnsseed if both are provided"
(https://github.com/bitcoin/bitcoin/pull/28016)
(https://github.com/bitcoin/bitcoin/pull/28016)
🚀 achow101 merged a pull request: "doc: i2p: improve `-i2pacceptincoming` mention"
(https://github.com/bitcoin/bitcoin/pull/29813)
(https://github.com/bitcoin/bitcoin/pull/29813)