Skip to content

use new ExtranonceAllocator APIs#11

Merged
plebhash merged 5 commits into
stratum-mining:mainfrom
plebhash:2026-04-22-extranonce-allocator
Apr 28, 2026
Merged

use new ExtranonceAllocator APIs#11
plebhash merged 5 commits into
stratum-mining:mainfrom
plebhash:2026-04-22-extranonce-allocator

Conversation

@plebhash

@plebhash plebhash commented Apr 22, 2026

Copy link
Copy Markdown
Member

close #3


also removes the obsolete RequestedMaxTargetOutOfRange from stratum-mining/stratum#2118

@plebhash plebhash force-pushed the 2026-04-22-extranonce-allocator branch 3 times, most recently from 1169233 to 23b685d Compare April 24, 2026 15:14
@plebhash plebhash marked this pull request as ready for review April 24, 2026 15:14
@plebhash plebhash requested a review from Shourya742 April 24, 2026 15:14
@plebhash plebhash mentioned this pull request Apr 27, 2026
Merged

#[derive(uniffi::Object)]
pub struct Sv2ExtranoncePrefix {
inner: Mutex<Option<AllocatedExtranoncePrefix>>,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove Option here, and keep it just AllocatedExtranoncePrefix, and then pass it to new methods as owned, instead of having an Arc?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm I guess you want to bring the ownership pattern from stratum-mining/stratum#2098 into here (de-allocate automatically by dropping)

the main thing to keep in mind here is that UniFFI objects always cross the language boundary as handles/pointers, so they do not model move-only Rust ownership directly

I get how the Option seems a bit awkward at first, but we need a one-shot consume step on the Rust side, which is why the wrapper stores Option<AllocatedExtranoncePrefix> and uses take_inner() to consume it exactly once.

if this were a pure-Rust API, passing AllocatedExtranoncePrefix by value would make sense, but with the current UniFFI object boundary that would require a broader API redesign (e.g.: Sv2ExtranonceAllocator instantiating channels, which tbh feels like a terrible idea).

I'm adding comments to explain the reasoning behind Option, but I would prefer that we avoid broader API redesigns.

self.extranonce_allocator = Sv2ExtranonceAllocator(
local_prefix_bytes=static_prefix,
total_extranonce_len=FULL_EXTRANONCE_SIZE,
max_channels=MAX_CHANNELS,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very interesting that CI didn't flagged this

Suggested change
max_channels=MAX_CHANNELS,
max_channels=POOL_MAX_CHANNELS,

@plebhash plebhash Apr 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed and added 9801d35 to run some examples on CI

full example coverage on CI is non-trivial and out of scope

@plebhash plebhash force-pushed the 2026-04-22-extranonce-allocator branch 2 times, most recently from 066e07f to 9801d35 Compare April 28, 2026 17:26

@Shourya742 Shourya742 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can merge after CI is fixed

@plebhash plebhash force-pushed the 2026-04-22-extranonce-allocator branch from 9801d35 to 6132d70 Compare April 28, 2026 17:59
@plebhash plebhash merged commit 0c1f2de into stratum-mining:main Apr 28, 2026
2 checks passed
@plebhash plebhash deleted the 2026-04-22-extranonce-allocator branch April 28, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

need to adapt to new channels_sv2::extranonce_manager APIs

2 participants