💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2085526718)
Remember that an `Assume()` is **never** necessary. Its only purpose is helping you, the reviewer, gain confidence that a property is likely true.
As for what it isn't here, I didn't think it'd be useful, as `ClearChunkData` is a very low-level internal function. I put the `Assume()` calls around maintenance largely in higher-level functions, but if you feel it would help you to have more, feel free to suggest them.
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2085526718)
Remember that an `Assume()` is **never** necessary. Its only purpose is helping you, the reviewer, gain confidence that a property is likely true.
As for what it isn't here, I didn't think it'd be useful, as `ClearChunkData` is a very low-level internal function. I put the `Assume()` calls around maintenance largely in higher-level functions, but if you feel it would help you to have more, feel free to suggest them.
💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2085528194)
I prefer avoiding early returns if they don't really simplify the code.
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2085528194)
I prefer avoiding early returns if they don't really simplify the code.
💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2085520213)
> This is pretty much identical to the logic in CompareMainOrder, should this be encapsulated in a single place as the source of truth (rather than having to ensure they stay equivalent)?
Good idea, done.
> Unrelated to this particular PR, but what is the intended caller/use case for CompareMainOrder? Is the reason CompareMainOrder calls MakeAcceptable while ChunkOrder does not simply because we assume its already been called when ChunkOrder is invoked?
`ChunkOrder`'s comparison functio
...
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2085520213)
> This is pretty much identical to the logic in CompareMainOrder, should this be encapsulated in a single place as the source of truth (rather than having to ensure they stay equivalent)?
Good idea, done.
> Unrelated to this particular PR, but what is the intended caller/use case for CompareMainOrder? Is the reason CompareMainOrder calls MakeAcceptable while ChunkOrder does not simply because we assume its already been called when ChunkOrder is invoked?
`ChunkOrder`'s comparison functio
...
💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#issuecomment-2874185965)
Changes made to address @monlovesmango's comments:
```diff
--- a/src/txgraph.cpp
+++ b/src/txgraph.cpp
@@ -282,6 +282,27 @@ private:
return a->m_sequence <=> b->m_sequence;
}
+ /** Compare two entries (which must both exist within the main graph). */
+ std::strong_ordering CompareMainTransactions(GraphIndex a, GraphIndex b) const noexcept
+ {
+ Assume(a < m_entries.size() && b < m_entries.size());
+ const auto& entry_a = m_entries[a];
+
...
(https://github.com/bitcoin/bitcoin/pull/31444#issuecomment-2874185965)
Changes made to address @monlovesmango's comments:
```diff
--- a/src/txgraph.cpp
+++ b/src/txgraph.cpp
@@ -282,6 +282,27 @@ private:
return a->m_sequence <=> b->m_sequence;
}
+ /** Compare two entries (which must both exist within the main graph). */
+ std::strong_ordering CompareMainTransactions(GraphIndex a, GraphIndex b) const noexcept
+ {
+ Assume(a < m_entries.size() && b < m_entries.size());
+ const auto& entry_a = m_entries[a];
+
...
💬 ariel-lore commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2874205808)
Concept NACK
Deprecating the feature is an indication that the feature could be removed in the future without discussion. It is not different enough to just removing the feature entirely today, thus is isn't a compromise. Some are saying that features can stay deprecated for years but that's a false equivalence because those features are not as controversial as the OP_RETURN limit.
Increasing the default OP_RETURN limit to 100,000 is also contentious and should not be merged without furthe
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2874205808)
Concept NACK
Deprecating the feature is an indication that the feature could be removed in the future without discussion. It is not different enough to just removing the feature entirely today, thus is isn't a compromise. Some are saying that features can stay deprecated for years but that's a false equivalence because those features are not as controversial as the OP_RETURN limit.
Increasing the default OP_RETURN limit to 100,000 is also contentious and should not be merged without furthe
...
💬 theStack commented on pull request "test: refactor: overhaul (w)txid determination for `CTransaction` objects":
(https://github.com/bitcoin/bitcoin/pull/32421#issuecomment-2874221345)
Rebased on master (needed due to a few conflicts in functional tests after #32155 was merged).
(https://github.com/bitcoin/bitcoin/pull/32421#issuecomment-2874221345)
Rebased on master (needed due to a few conflicts in functional tests after #32155 was merged).
⚠️ Pamela001001 opened an issue: "HOW TO FIND A LEGITIMATE CRYPTO RECOVERY EXPERT//PASSCODE CYBER RECOVERY"
(https://github.com/bitcoin/bitcoin/issues/32474)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
PASSCODE CYBER RECOVERY
WhatsApp: +1(647)399-4074
Telegram : @passcodecyberrecovery
A clip titled “Joe Rogan Reveals Secret Ethereum Fork” autopay on YouTube, featuring Rogan’s unmistakable voice praising a supposed “ETH 2.0 upgrade” that promised early investors up to 10x returns. Everything about the video seemed authentic, Rogan's casual tone, the familiar podcast studio, and even comm
...
(https://github.com/bitcoin/bitcoin/issues/32474)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
PASSCODE CYBER RECOVERY
WhatsApp: +1(647)399-4074
Telegram : @passcodecyberrecovery
A clip titled “Joe Rogan Reveals Secret Ethereum Fork” autopay on YouTube, featuring Rogan’s unmistakable voice praising a supposed “ETH 2.0 upgrade” that promised early investors up to 10x returns. Everything about the video seemed authentic, Rogan's casual tone, the familiar podcast studio, and even comm
...
💬 Pamela001001 commented on issue "HOW TO FIND A LEGITIMATE CRYPTO RECOVERY EXPERT//PASSCODE CYBER RECOVERY":
(https://github.com/bitcoin/bitcoin/issues/32474#issuecomment-2874355189)
HOW TO FIND A LEGITIMATE CRYPTO RECOVERY EXPERT//PASSCODE CYBER RECOVERY
PASSCODE CYBER RECOVERY
WhatsApp: +1(647)399-4074
Telegram : @passcodecyberrecovery
A clip titled “Joe Rogan Reveals Secret Ethereum Fork” autopay on YouTube, featuring Rogan’s unmistakable voice praising a supposed “ETH 2.0 upgrade” that promised early investors up to 10x returns. Everything about the video seemed authentic, Rogan's casual tone, the familiar podcast studio, and even commentary from a so-called “blockchai
...
(https://github.com/bitcoin/bitcoin/issues/32474#issuecomment-2874355189)
HOW TO FIND A LEGITIMATE CRYPTO RECOVERY EXPERT//PASSCODE CYBER RECOVERY
PASSCODE CYBER RECOVERY
WhatsApp: +1(647)399-4074
Telegram : @passcodecyberrecovery
A clip titled “Joe Rogan Reveals Secret Ethereum Fork” autopay on YouTube, featuring Rogan’s unmistakable voice praising a supposed “ETH 2.0 upgrade” that promised early investors up to 10x returns. Everything about the video seemed authentic, Rogan's casual tone, the familiar podcast studio, and even commentary from a so-called “blockchai
...
✅ fanquake closed an issue: "HOW TO FIND A LEGITIMATE CRYPTO RECOVERY EXPERT//PASSCODE CYBER RECOVERY"
(https://github.com/bitcoin/bitcoin/issues/32474)
(https://github.com/bitcoin/bitcoin/issues/32474)
💬 FractalEncrypt commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2874362210)
**Difficulty creating transactions with multiple OP_RETURN outputs for testing PR #32406**
I apologize in advance if this is not the right place to ask/post this, I am relatively inexperienced, but trying my best to help.
I'm working on testing PR #32406 and specifically want to create transactions on regtest with multiple large OP_RETURN outputs to stress-test the new functionality. I'm running a build of this PR on Ubuntu (via WSL2).
My goal is to programmatically (ideally via Pytho
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2874362210)
**Difficulty creating transactions with multiple OP_RETURN outputs for testing PR #32406**
I apologize in advance if this is not the right place to ask/post this, I am relatively inexperienced, but trying my best to help.
I'm working on testing PR #32406 and specifically want to create transactions on regtest with multiple large OP_RETURN outputs to stress-test the new functionality. I'm running a build of this PR on Ubuntu (via WSL2).
My goal is to programmatically (ideally via Pytho
...
💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085633551)
This is an error and should return non-zero exit code.
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085633551)
This is an error and should return non-zero exit code.
💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085634531)
This function is really bad and should've been deprecated, you should use `try_exists` instead.
But also, why do this extra check? Just `read` already does the same thing with less code and less problems.
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085634531)
This function is really bad and should've been deprecated, you should use `try_exists` instead.
But also, why do this extra check? Just `read` already does the same thing with less code and less problems.
💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085633049)
Have you tested that this picks the right one on Windows? That may be a good reason for the attribute.
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085633049)
Have you tested that this picks the right one on Windows? That may be a good reason for the attribute.
💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085643521)
This loads the entire block file into memory. I'm not sure if such memory usage is desirable.
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085643521)
This loads the entire block file into memory. I'm not sure if such memory usage is desirable.
💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085637066)
Why though? After "unxoring" the magic bytes just will be the same. Anyway, might still be a good idea to mask it better.
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085637066)
Why though? After "unxoring" the magic bytes just will be the same. Anyway, might still be a good idea to mask it better.
💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085640823)
Why not reuse the file handle later and perhaps take advantage of `BufReader`?
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085640823)
Why not reuse the file handle later and perhaps take advantage of `BufReader`?
💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085638849)
Why not `path.display()`?
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085638849)
Why not `path.display()`?
💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085644155)
Yeah, Rust even has `chunks_exact` method for this (and the rest can be xored after the loop). It should also auto-vectorize.
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085644155)
Yeah, Rust even has `chunks_exact` method for this (and the rest can be xored after the loop). It should also auto-vectorize.
💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085644865)
Cool, this is almost atomic. I say almost because you're not calling `sync_data` (because you're not using file handle - I think you should).
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085644865)
Cool, this is almost atomic. I say almost because you're not calling `sync_data` (because you're not using file handle - I think you should).
💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085638116)
BTW, this is not atomic. Does it matter? I don't really know. Ideally the tool could continue if it fails but that's not easy.
Still, at least making sure this one doesn't get corrupted may be important?
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2085638116)
BTW, this is not atomic. Does it matter? I don't really know. Ideally the tool could continue if it fails but that's not easy.
Still, at least making sure this one doesn't get corrupted may be important?