Skip to content

Conversation

@GitGab19
Copy link
Member

@GitGab19 GitGab19 commented Nov 3, 2025

This PR clarifies the usage of extension_type field in message frames, as discussed over discord.

cc @TheBlueMatt

@GitGab19 GitGab19 force-pushed the clarify-extension-type branch 2 times, most recently from 71f8dba to 35ff875 Compare November 3, 2025 17:20
Copy link
Contributor

@Shourya742 Shourya742 left a comment

Choose a reason for hiding this comment

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

Some nits I observed while reading the extension docs, can be taken up in this PR or some other PR.

  1. unsupported type is a list:

    Client <--- RequestExtensions.Error [unsupported: 0x0002, required: []] ---- Server
    , we just show a value here, not list.

  2. Both client extensions request are unsupported:

    Client <--- RequestExtensions.Error [unsupported: 0x0003, required: 0x0005] ---- Server
    , we have only shown one.

  3. This needs to be changed:

    Client <--- RequestExtensions.Error [0x0002] ---- Server
    , not correct format of RequestExtension.Error

  4. New line missing here:

    After negotiation, the client appends TLV fields at the end of `SubmitSharesExtended`.

  5. We are not including the tlv inside the message, but outside its frame boundary:

    - The `user_identity` field **MUST** be included in `SubmitSharesExtended` as a TLV with Type `0x0002`.
    , this needs to be elaborated.

@GitGab19 GitGab19 force-pushed the clarify-extension-type branch from 35ff875 to bae935f Compare November 24, 2025 15:57
@GitGab19 GitGab19 force-pushed the clarify-extension-type branch from bae935f to 83230b2 Compare November 24, 2025 17:38
@GitGab19 GitGab19 force-pushed the clarify-extension-type branch from 83230b2 to 2cba36a Compare November 24, 2025 17:39
@plebhash plebhash merged commit 3012409 into stratum-mining:main Nov 24, 2025
1 check passed
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.

3 participants