Skip to content

allow access to all sv1 message props#2224

Open
coleFD wants to merge 2 commits into
stratum-mining:mainfrom
coleFD:public-sv1-message-props
Open

allow access to all sv1 message props#2224
coleFD wants to merge 2 commits into
stratum-mining:mainfrom
coleFD:public-sv1-message-props

Conversation

@coleFD

@coleFD coleFD commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

make fields in sv1 messages public

Comment thread sv1/src/methods/server_to_client.rs Outdated

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.

missing this one

is that intentional, or not needed for your use-case @coleFD ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

nope just missed it

Comment on lines 681 to 684
pub struct VersionRollingParams {
mask: Option<HexU32Be>,
min_bit_count: Option<HexU32Be>,
pub mask: Option<HexU32Be>,
pub min_bit_count: Option<HexU32Be>,
}

@plebhash plebhash Jul 9, 2026

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.

making these fields public means VersionRollingParams can now be instantiated with combinations we currently treat as invalid when parsing.

for example:

VersionRollingParams { mask: None, min_bit_count: Some(HexU32Be(0x10)) }

ConfigureExtension::from_value explicitly rejects this shape ((None, Some(_)) => Err(...)).

so I wonder whether it wouldn't be safer to change the PR approach, while keeping internal values private and exposing getters instead?

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.

2 participants