Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

need to cleanup JD Protocol specs #77

Open
plebhash opened this issue May 14, 2024 · 8 comments · May be fixed by #104
Open

need to cleanup JD Protocol specs #77

plebhash opened this issue May 14, 2024 · 8 comments · May be fixed by #104
Assignees

Comments

@plebhash
Copy link
Contributor

plebhash commented May 14, 2024

JD Protocol spec is somewhat outdated.

Some things to be done:

  • remove mentions to terms that are no longer part of the protocol (e.g.: CreateMiningJob, Job Selector, Job Distribution Protocol)
  • add context on possible different setups
  • add formal definition of JDS + JDC
  • add missing description to some messages
  • clarify definition on some messages
@plebhash
Copy link
Contributor Author

I have some drafts so I'd like to assign this to myself, but it seems I don't have rights on this repo.

@pavlenex can you assign this to me please?

@plebhash
Copy link
Contributor Author

@plebhash

This comment was marked as outdated.

@GitGab19
Copy link
Collaborator

GitGab19 commented Jul 3, 2024

Here a suggestion for the SubmitSolution message description:

"Sent by the client as soon as a valid block is found, so that it can be propagated also by the server, which is directly connected to its own Bitcoin node. In the meantime the block is transmitted to the network by the client as well, through the SubmitSolution defined in Template-Distribution-Protocol. In this way a valid solution is immediately propagated on both client and server sides."

@plebhash let me know if you think it's enough or not

@plebhash
Copy link
Contributor Author

plebhash commented Jul 3, 2024

Here a suggestion for the SubmitSolution message description:

"Sent by the client as soon as a valid block is found, so that it can be propagated also by the server, which is directly connected to its own Bitcoin node. In the meantime the block is transmitted to the network by the client as well, through the SubmitSolution defined in Template-Distribution-Protocol. In this way a valid solution is immediately propagated on both client and server sides."

@plebhash let me know if you think it's enough or not

looks good, thanks!

whenever we're working on the PR for this issue, I'd also add something like: "This message doesn't elicit any Success or Error response, so it works as fire-and-forget"

@plebhash
Copy link
Contributor Author

there's an outdated reference to Job Distribution Protocol on the protocol field of SetupConnection

https://stratumprotocol.org/specification/03-Protocol-Overview/#361-setupconnection-client---server

@plebhash plebhash changed the title cleanup / rewrite JD Protocol specs cleanup JD Protocol specs Aug 29, 2024
@plebhash plebhash changed the title cleanup JD Protocol specs need to cleanup JD Protocol specs Sep 5, 2024
@plebhash plebhash linked a pull request Oct 26, 2024 that will close this issue
@GitGab19
Copy link
Collaborator

@plebhash we need to fix the description of coinbase_tx_prefix field in DeclareMiningJob message: https://github.com/stratum-mining/sv2-spec/blob/main/06-Job-Declaration-Protocol.md#614-declareminingjob-client---server

@plebhash
Copy link
Contributor Author

plebhash commented Oct 28, 2024

@plebhash we need to fix the description of coinbase_tx_prefix field in DeclareMiningJob message: https://github.com/stratum-mining/sv2-spec/blob/main/06-Job-Declaration-Protocol.md#614-declareminingjob-client---server

Actually it seems coinbase_tx_prefix and coinbase_tx_suffix are both wrong.

When compared to SetCustomMiningJob, they are repeating descriptions in a way that don't really make a lot of sense

DeclareMiningJob SetCustomMiningJob
coinbase_prefix / coinbase_tx_prefix The coinbase transaction nVersion field Up to 8 bytes (not including the length byte) which are to be placed at the beginning of the coinbase field in the coinbase transaction.
coinbase_suffix / coinbase_tx_suffix Up to 8 bytes (not including the length byte) which are to be placed at the beginning of the coinbase field in the coinbase transaction.
coinbase_tx_version The coinbase transaction nVersion field

I digged into the file history it seems these errors were likely unintentionally introduced via 9b0e40b while @Fi3 was editing the table.

By looking at this commit, I can infer that coinbase_tx_prefix can be easily reverted its original description:

Up to 8 bytes (not including the length byte) which are to be placed at the beginning of the coinbase field in the coinbase transaction

But its not clear to me what should be written as a description to coinbase_tx_suffix?

If we take NewExtendedMiningJob as a reference, where there's a field with the same name, it's description is:

Suffix part of the coinbase transaction

unfortunately that is not too insightful.

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 a pull request may close this issue.

2 participants