💬 l0rinc commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2190812498)
The PR description still states:
> Both the quota and time window are configurable
As far as I can tell that's not the case anymore.
> When logging is restarted a tally of how many messages/bytes were dropped is printed
Do we also print the message count?
----
> But if we stay with the per-line logging-quota, I think this should at least be explained in the PR description
I meant the reason for choosing to restrict based on source lines and not globally. I'm still not fully co
...
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2190812498)
The PR description still states:
> Both the quota and time window are configurable
As far as I can tell that's not the case anymore.
> When logging is restarted a tally of how many messages/bytes were dropped is printed
Do we also print the message count?
----
> But if we stay with the per-line logging-quota, I think this should at least be explained in the PR description
I meant the reason for choosing to restrict based on source lines and not globally. I'm still not fully co
...
💬 davidgumberg commented on pull request "p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#issuecomment-3046200634)
> Not sure I understand this - why would the block be part of the active chain by that time? We started a request in the first place because we don't know the full block yet, and if that request has timed out, chances are we still don't have the block, in which case it won't be part of our active chain. Or do you count on a parallel compact-block download being successful in the meantime?
My assumption was that it was pretty unlikely for it to ever take >= 10 minutes to receive a block after
...
(https://github.com/bitcoin/bitcoin/pull/32869#issuecomment-3046200634)
> Not sure I understand this - why would the block be part of the active chain by that time? We started a request in the first place because we don't know the full block yet, and if that request has timed out, chances are we still don't have the block, in which case it won't be part of our active chain. Or do you count on a parallel compact-block download being successful in the meantime?
My assumption was that it was pretty unlikely for it to ever take >= 10 minutes to receive a block after
...
💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3046202153)
>>Sending own transactions over long lived connections is problematic because then two different transactions that are otherwise unrelated, looking at the coins (ie on-chain analysis) could be linked together to the same originator.
>Is it not possible to perform this type of on-chain analysis even with privatebroadcast? For example, someone could observe the blockchain and fingerprint transactions as likely coming from a particular node and label it as "vasilsnode"?
Consider a wallet with
...
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3046202153)
>>Sending own transactions over long lived connections is problematic because then two different transactions that are otherwise unrelated, looking at the coins (ie on-chain analysis) could be linked together to the same originator.
>Is it not possible to perform this type of on-chain analysis even with privatebroadcast? For example, someone could observe the blockchain and fingerprint transactions as likely coming from a particular node and label it as "vasilsnode"?
Consider a wallet with
...
💬 l0rinc commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2190813991)
Ok, if you're fine with that, I'm fine with that, let's do the remaining ones in follow-ups.
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2190813991)
Ok, if you're fine with that, I'm fine with that, let's do the remaining ones in follow-ups.
💬 l0rinc commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2190816506)
Fair points, maybe we could hint at that in the first message which announces that some logs are being suppressed (explaining what the new `*` prefix indicates).
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2190816506)
Fair points, maybe we could hint at that in the first message which announces that some logs are being suppressed (explaining what the new `*` prefix indicates).
💬 l0rinc commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2190822287)
> Recursion is not inherently bad
When the stopping condition isn't rock solit, it kinda' is. Maybe it is in this case - but given that we had to add a commit to reassure the readers, it may not be. I'd sleep better knowing that we haven't just introduced a new attack vector...
> What do you suggest?
In other cases we've switched recursive methods to iterative ones, e.g. https://github.com/bitcoin/bitcoin/pull/32351. I haven't investigated what that would imply here, got already tired a
...
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2190822287)
> Recursion is not inherently bad
When the stopping condition isn't rock solit, it kinda' is. Maybe it is in this case - but given that we had to add a commit to reassure the readers, it may not be. I'd sleep better knowing that we haven't just introduced a new attack vector...
> What do you suggest?
In other cases we've switched recursive methods to iterative ones, e.g. https://github.com/bitcoin/bitcoin/pull/32351. I haven't investigated what that would imply here, got already tired a
...
👍 l0rinc approved a pull request: "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError"
(https://github.com/bitcoin/bitcoin/pull/32604#pullrequestreview-2995010190)
While there are still more nits left than I'm comfortable with, I understand wanting to have some progress - and the current PR is indeed an improvement.
Lightly tested code review ACK f47e2ea9137c3a832e07d6dd845c55d35d533fa9
I'm looking forward to ironing out the wrinkles in follow-up PRs.
(https://github.com/bitcoin/bitcoin/pull/32604#pullrequestreview-2995010190)
While there are still more nits left than I'm comfortable with, I understand wanting to have some progress - and the current PR is indeed an improvement.
Lightly tested code review ACK f47e2ea9137c3a832e07d6dd845c55d35d533fa9
I'm looking forward to ironing out the wrinkles in follow-up PRs.
💬 l0rinc commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-3046267831)
re-ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-3046267831)
re-ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2190863515)
> As far as I can tell that's not the case anymore.
Ah, this was confusing. I meant to say that `LogRateLimiter` is parameterized, but it reads like a node operator is able to configure the quote and time window without modifying the source code. I've removed this line since it's confusing.
> Do we also print the message count?
Removed mention of "messages".
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2190863515)
> As far as I can tell that's not the case anymore.
Ah, this was confusing. I meant to say that `LogRateLimiter` is parameterized, but it reads like a node operator is able to configure the quote and time window without modifying the source code. I've removed this line since it's confusing.
> Do we also print the message count?
Removed mention of "messages".
💬 l0rinc commented on pull request "doc: fix `BlockConnected` incorrect comment":
(https://github.com/bitcoin/bitcoin/pull/32893#issuecomment-3046308815)
Vector parameter was removed in https://github.com/bitcoin/bitcoin/commit/cdb893443cc16edf974f099b8485e04b3db1b1d7#diff-b169bd0694e8b0a87fd1097c8cfc1d6c8a926b49b375bb4141d37dcc624f975eL127-L131, the doc wasn't updated.
ACK 4e69aa5701a2dad3805ea26718e6a406adb8b748
(https://github.com/bitcoin/bitcoin/pull/32893#issuecomment-3046308815)
Vector parameter was removed in https://github.com/bitcoin/bitcoin/commit/cdb893443cc16edf974f099b8485e04b3db1b1d7#diff-b169bd0694e8b0a87fd1097c8cfc1d6c8a926b49b375bb4141d37dcc624f975eL127-L131, the doc wasn't updated.
ACK 4e69aa5701a2dad3805ea26718e6a406adb8b748
💬 luke-jr commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2190894052)
But `block` isn't a pointer...?
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2190894052)
But `block` isn't a pointer...?
💬 luke-jr commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2190896199)
This is just a move, so seems out of scope
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2190896199)
This is just a move, so seems out of scope
💬 luke-jr commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2190899759)
An attempt to have a somewhat optional extension, but given the need to prove wtxid for non-witness txs, maybe not the best approach.
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2190899759)
An attempt to have a somewhat optional extension, but given the need to prove wtxid for non-witness txs, maybe not the best approach.
📝 achow101 opened a pull request: "wallet: Prepare for future upgrades by recording versions of last client to open and decrypt"
(https://github.com/bitcoin/bitcoin/pull/32895)
When a wallet is automatically upgraded, we always do so in a way that allows the user to load their wallet in an older version. When the user loads their wallet into an upgraded version again, the wallet may not perform the automatic upgrade and end up with upgraded and non-upgraded material in the wallet.
If we write the version of the last client to open the wallet, we would be able to detect these upgrade-downgrade-upgrade situations and perform another upgrade if so. However, in order fo
...
(https://github.com/bitcoin/bitcoin/pull/32895)
When a wallet is automatically upgraded, we always do so in a way that allows the user to load their wallet in an older version. When the user loads their wallet into an upgraded version again, the wallet may not perform the automatic upgrade and end up with upgraded and non-upgraded material in the wallet.
If we write the version of the last client to open the wallet, we would be able to detect these upgrade-downgrade-upgrade situations and perform another upgrade if so. However, in order fo
...
📝 ishaanam opened a pull request: "wallet, rpc: add v3 transaction creation and wallet support"
(https://github.com/bitcoin/bitcoin/pull/32896)
This PR Implements the following:
- If creating a v3 transaction, `AvailableCoins` doesn't return unconfirmed v2 utxos (and vice versa)
- `AvailableCoins` doesn't return an unconfirmed v3 utxo if its transaction already has a child
- If a v3 transaction is kicked out of the mempool by a sibling, mark the sibling as a mempool conflict
- Throw an error if pre-selected inputs are of the wrong transaction version
- Allow setting version to 3 manually in `createrawtransaction` (uses commits from
...
(https://github.com/bitcoin/bitcoin/pull/32896)
This PR Implements the following:
- If creating a v3 transaction, `AvailableCoins` doesn't return unconfirmed v2 utxos (and vice versa)
- `AvailableCoins` doesn't return an unconfirmed v3 utxo if its transaction already has a child
- If a v3 transaction is kicked out of the mempool by a sibling, mark the sibling as a mempool conflict
- Throw an error if pre-selected inputs are of the wrong transaction version
- Allow setting version to 3 manually in `createrawtransaction` (uses commits from
...
🚀 glozow merged a pull request: "cluster mempool: add TxGraph reorg functionality"
(https://github.com/bitcoin/bitcoin/pull/31553)
(https://github.com/bitcoin/bitcoin/pull/31553)
⚠️ davidtaubmann opened an issue: "Bitcoin Px (Fork focused on Persons, remove institutions)"
(https://github.com/bitcoin/bitcoin/issues/32897)
### Please describe the feature you'd like to see added.
In this new fork, only one wallet could be created for each physical persona, probably using a proof like the national identity number "[NIN](https://en.wikipedia.org/wiki/National_identification_number)" (e.g.: US+SSN, MX+CURP, etc.), and getting sure it's a living person (not yet registered as death).
To be able to carry out transactions with institutions, this could be done, through decentralized exchanges (DEXs).
To perform the for
...
(https://github.com/bitcoin/bitcoin/issues/32897)
### Please describe the feature you'd like to see added.
In this new fork, only one wallet could be created for each physical persona, probably using a proof like the national identity number "[NIN](https://en.wikipedia.org/wiki/National_identification_number)" (e.g.: US+SSN, MX+CURP, etc.), and getting sure it's a living person (not yet registered as death).
To be able to carry out transactions with institutions, this could be done, through decentralized exchanges (DEXs).
To perform the for
...
✅ pinheadmz closed an issue: "Bitcoin Px (Fork focused on Persons, remove institutions)"
(https://github.com/bitcoin/bitcoin/issues/32897)
(https://github.com/bitcoin/bitcoin/issues/32897)
💬 pinheadmz commented on issue "Bitcoin Px (Fork focused on Persons, remove institutions)":
(https://github.com/bitcoin/bitcoin/issues/32897#issuecomment-3046418540)
This belongs on the Bitcoin dev mailing list or maybe delving Bitcoin, not the issue tracker for this software project.
(https://github.com/bitcoin/bitcoin/issues/32897#issuecomment-3046418540)
This belongs on the Bitcoin dev mailing list or maybe delving Bitcoin, not the issue tracker for this software project.
💬 glozow commented on pull request "rpc: use anti-fee-sniping in send, sendall and walletcreatefundedpsbt":
(https://github.com/bitcoin/bitcoin/pull/32892#issuecomment-3046428650)
Any chance this is a duplicate of #28944?
(https://github.com/bitcoin/bitcoin/pull/32892#issuecomment-3046428650)
Any chance this is a duplicate of #28944?