Add extranonce_manager module in channels_sv2#2098
Add extranonce_manager module in channels_sv2#2098GitGab19 wants to merge 5 commits intostratum-mining:mainfrom
extranonce_manager module in channels_sv2#2098Conversation
b33c8be to
834b6ce
Compare
834b6ce to
7394b40
Compare
7394b40 to
6038120
Compare
39adc06 to
eb4f941
Compare
|
@pavlenex had some issues while trying to connect rented hashrate from Nicehash to a tProxy (exposed over internet)
while reviewing/testing this PR, we should make sure nicehash use-case is not blocked |
|
the suffix for example:
we should establish terminology that clearly separates concerns. I suggest we rename variables to something like:
on the suggestion above, I'm leaving what's currently described as IMO this would bring more consistency to how the new concepts map into the code, and make the code easier to reason about and maintain on the long term. |
|
aside from the suggestion above, I think I got a much better understanding of this new approach I'll make another round of review tomorrow (my clanker found some potential edge cases that could cause problems, but I need to sanity-check them before posting here) |
9d90704 to
e590b32
Compare
|
@plebhash I included all your suggestions and now we have:
|
02e4ebc to
efb3a27
Compare
efb3a27 to
4395233
Compare
|
semver concern coming from my clanker:
I think this is less of a concern if consumers only used but if anyone imports speaking more from the human side, I empathize with the psychological urge to be conservative and avoid major bumps as much as possible, especially when due to indirect changes on the dependency chain however, I think this is more of an undesired consequence of the way SRI core crates were structured (excessive crate fragmentation), and I've somewhat accepted the tradeoff of living with frequent major bumps (in name of avoiding forcing dependency hell on downstream consumers) |
|
Should we start bumping MAJOR on every dependent crate though? |
hmm that feels too generic... v0.x.y cratesHere's a table my agent to put together, which after careful examination, I agree with:
Those crates are still on dependencies that don't touch public APIsLet's say crate B depends on crate A, which is undergoing some breaking change. The main criterion is whether B public API has any types from A. Not whether the breaking change in A leaks over B in any significant way. If B only uses types from A without exposure via public APIs, then a PATCH bump would be fine. More specifically with the case of this PR:
if there were no types from PR #2133 is formalizing these rules into |
|
my final review point on this PR is somewhat subjective, and therefore it's not blocking I feel we could have better conceptual organization by splitting into submodules
commit c89e150 implements these suggestions, and could be either cherry-picked or squashed into other commits on this PR if agreed upon if not (especially given this could also affect stratum-mining/sv2-apps#310), I think this PR is already good to go (aside from the versioning issues detailed above) |
|
about the Nicehash comment above: #2098 (comment) I haven't had time to do some actual testing with this PR + stratum-mining/sv2-apps#310, but from what I understood of this PR on a conceptual level, I don't see any major blockers at
both points seem sufficiently covered with these new APIs, and any potential edge cases with Sv1 extranonce1 or extranonce2 could only arise from specific combinations of configuration parameters of Pool + tProxy TLDR I don't see any material blockers on this PR with regards to the issues observed on Nicehash, at least on a conceptual level |

This PR introduces a new
extranonce_managermodule inchannels_sv2centered aroundExtranonceAllocatorIt's a unified, reusable allocator for extranonce prefixes for both standard and extended channels.
It replaces the legacy
ExtendedExtranoncefactory frommining_sv2and provides a clearer, spec‑aligned, and more configurable way to size and manage extranonce space across pools, proxies (JDC/Translator), and other applications.Internally, the allocator uses a compact bitmap (1 bit per channel) to track local_prefix_id usage, ensuring fast allocations, deterministic reuse when channels close, and no overlap between standard and extended channel prefixes.
This PR is inspired by the @coleFD's bitvec approach (see #2049), but it doesn't add any new external dependencies to our crates.
Closes #1924
companion stratum-mining/sv2-apps#310