💬 fjahr commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1225871000)
nit: Developer notes say "Class member variables have a `m_` prefix.".
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1225871000)
nit: Developer notes say "Class member variables have a `m_` prefix.".
💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1225913119)
latest push collects stats as a `CConnman` member, so we don't have to worry about iterating through all nodes
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1225913119)
latest push collects stats as a `CConnman` member, so we don't have to worry about iterating through all nodes
💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1225913181)
updated to incorporate your suggestions, thanks!
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1225913181)
updated to incorporate your suggestions, thanks!
💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1225913245)
moved to member to prevent iterating through all nodes
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1225913245)
moved to member to prevent iterating through all nodes
💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1225913270)
done, thanks!
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1225913270)
done, thanks!
💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1225913626)
we have now removed any clearnet bundling, so this quirk is no longer relevant
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1225913626)
we have now removed any clearnet bundling, so this quirk is no longer relevant
💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1225914692)
question: I noticed you used array in your implementation but we were using vector. is there a strong reason for that?
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1225914692)
question: I noticed you used array in your implementation but we were using vector. is there a strong reason for that?
💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1586321519)
thank you for the reviews @ajtowns & @vasild 🙌
significant changes with latest push:
* introduce a new member on `CConnman` called `m_network_conn_counts` which is a map from `Network` to an atomic that keeps track of the number of `OUTBOUND_FULL_RELAY` and `MANUAL` connections on the node broken down by network. this means we don't have to iterate through all nodes anymore.
* changed the interval from 1 min to 5 min.
* got rid of any "clearnet" bundling, now we treat IPV4 & IPV6 as s
...
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1586321519)
thank you for the reviews @ajtowns & @vasild 🙌
significant changes with latest push:
* introduce a new member on `CConnman` called `m_network_conn_counts` which is a map from `Network` to an atomic that keeps track of the number of `OUTBOUND_FULL_RELAY` and `MANUAL` connections on the node broken down by network. this means we don't have to iterate through all nodes anymore.
* changed the interval from 1 min to 5 min.
* got rid of any "clearnet" bundling, now we treat IPV4 & IPV6 as s
...
💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1225915157)
updated to have a small connman member to keep track of counts. now we don't iterate through all peers anymore :)
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1225915157)
updated to have a small connman member to keep track of counts. now we don't iterate through all peers anymore :)
💬 Giszmo commented on issue "Indefinite "Bitcoin Core is shutting down..."":
(https://github.com/bitcoin/bitcoin/issues/27848#issuecomment-1586325943)
So I killed it, started it, waited to catch up and now Exit took some 5 or so seconds.
I suspect that the issue might have to do with flaky internet.
(https://github.com/bitcoin/bitcoin/issues/27848#issuecomment-1586325943)
So I killed it, started it, waited to catch up and now Exit took some 5 or so seconds.
I suspect that the issue might have to do with flaky internet.
💬 LarryRuane commented on pull request "test: allow BITCOIN_TEST_PATH to specify working dir":
(https://github.com/bitcoin/bitcoin/pull/26564#issuecomment-1586362907)
Rebased for merge conflict.
(https://github.com/bitcoin/bitcoin/pull/26564#issuecomment-1586362907)
Rebased for merge conflict.
💬 sdaftuar commented on pull request "p2p: Stop relaying non-mempool txs":
(https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1586454719)
ACK faa2976a56ea7cdfd77ce2580a89ce493b57b5d4
(https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1586454719)
ACK faa2976a56ea7cdfd77ce2580a89ce493b57b5d4
💬 ajtowns commented on pull request "p2p: Stop relaying non-mempool txs":
(https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1586490389)
ACK faa2976a56ea7cdfd77ce2580a89ce493b57b5d4
(https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1586490389)
ACK faa2976a56ea7cdfd77ce2580a89ce493b57b5d4
💬 ajtowns commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1586532229)
> > without #26711 we are still letting things into mempool that are possibly problematic, no?
>
> You're right. Every time we think about it a bit longer, another not-quite-ideal edge case comes up. Especially with full mempools and eviction being something to consider.
I think this should have remained as a separate PR, and should have been okay to merge as-is, rather than adding only loosely-related commits to #26711. Two reasons:
- after #26933 this doesn't allow any problematic be
...
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1586532229)
> > without #26711 we are still letting things into mempool that are possibly problematic, no?
>
> You're right. Every time we think about it a bit longer, another not-quite-ideal edge case comes up. Especially with full mempools and eviction being something to consider.
I think this should have remained as a separate PR, and should have been okay to merge as-is, rather than adding only loosely-related commits to #26711. Two reasons:
- after #26933 this doesn't allow any problematic be
...
💬 ajtowns commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1226035519)
Should probably mark it as EXPERIMENTAL in the rpc docs -- see "sendall" eg in wallet/rpc/spend.cpp
Not really sure this should be advertised in the release notes at this stage at all, though.
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1226035519)
Should probably mark it as EXPERIMENTAL in the rpc docs -- see "sendall" eg in wallet/rpc/spend.cpp
Not really sure this should be advertised in the release notes at this stage at all, though.
💬 ajtowns commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1226063546)
This check doesn't feel like it makes sense to me -- it should either be unnecessary (because dealing with subpackages first takes care of it), or else it seems insufficient (because a grandparent might be paying for parent and child, and child paying for parent; but grandparents alone are good enough, but after they're accepted parent and child combined aren't good enough).
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1226063546)
This check doesn't feel like it makes sense to me -- it should either be unnecessary (because dealing with subpackages first takes care of it), or else it seems insufficient (because a grandparent might be paying for parent and child, and child paying for parent; but grandparents alone are good enough, but after they're accepted parent and child combined aren't good enough).
💬 ajtowns commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1226067022)
I worry that this approach can cause O(n^2) validation costs in some way, if you've got n txs and end up doing work that results in multiple soft-rejects for each tx.
I wonder if it wouldn't be better to do something more like:
* topologically sort the package
* accept each tx in order into a mem-pond -- a special temporary mini pool just for this package, that enforces consensus rules, but doesn't care about minimum fees
* any txs that weren't valid obviously fail at this point and a
...
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1226067022)
I worry that this approach can cause O(n^2) validation costs in some way, if you've got n txs and end up doing work that results in multiple soft-rejects for each tx.
I wonder if it wouldn't be better to do something more like:
* topologically sort the package
* accept each tx in order into a mem-pond -- a special temporary mini pool just for this package, that enforces consensus rules, but doesn't care about minimum fees
* any txs that weren't valid obviously fail at this point and a
...
📝 Lildeebo2002 opened a pull request: "Update config.yml Dennis Louis Babcock jr added to Statoshinakomoto"
(https://github.com/bitcoin/bitcoin/pull/27858)
[Dennis Louis Babcock ](statoshinakomotod@gmail.com) Added creator name Dennis Louis Babcock jr Statoshinakomoto
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user
...
(https://github.com/bitcoin/bitcoin/pull/27858)
[Dennis Louis Babcock ](statoshinakomotod@gmail.com) Added creator name Dennis Louis Babcock jr Statoshinakomoto
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user
...
✅ fanquake closed a pull request: "Update config.yml Dennis Louis Babcock jr added to Statoshinakomoto"
(https://github.com/bitcoin/bitcoin/pull/27858)
(https://github.com/bitcoin/bitcoin/pull/27858)
📝 fanquake locked a pull request: "Update config.yml Dennis Louis Babcock jr added to Statoshinakomoto"
(https://github.com/bitcoin/bitcoin/pull/27858)
[Dennis Louis Babcock ](statoshinakomotod@gmail.com) Added creator name Dennis Louis Babcock jr Statoshinakomoto
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user
...
(https://github.com/bitcoin/bitcoin/pull/27858)
[Dennis Louis Babcock ](statoshinakomotod@gmail.com) Added creator name Dennis Louis Babcock jr Statoshinakomoto
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user
...