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

Remove short ids #1301

Draft
wants to merge 51 commits into
base: main
Choose a base branch
from
Draft

Remove short ids #1301

wants to merge 51 commits into from

Conversation

Fi3
Copy link
Collaborator

@Fi3 Fi3 commented Dec 17, 2024

As per stratum-mining/sv2-spec#99 this PR change the implementation so that in the JD protocol full tx ids are used instead of short tx ids. It also remove the IdentifyTransaction and IdentifyTransactionSuccess messages since are not needed anymore.

it need #1248

fi3 and others added 30 commits November 26, 2024 19:20
Coinbase signature is not part of the Sv2 protocol, some pool maybe want
to use it other not. The first part of the extranonce could also be
reserved for things that are not a pool signature. This pr rename the
pool_signature field of the channel factory into additional_coinbase_script_data
and change the type from Strgin to Vec<u8>, since can be anything.
The coinbase input script additional data should be sent as part of the
extranonce_prefix and not as part of the coinbase_prefix. So that a JDC
can see what the pool want as coinbase input script additional data
without the need to observ the coinbase prefix in job constructed by the
pool.
Update the pool to use an extranonce of 16 bytes rather then 32 so that
there is enaugh space to add the additional coinbase input script data.
Right now the channel factory only support one active job at time. That
means that if we receive a share for a job right after we sent downstream
a new job that share will be invalid. Now the channel factory keep track
of the last 3 jobs, so we give time to the dowstream to receive the job
and propagate it down before stop accepting shares for older job. This
is useful, and the system can be more responsive: as soon as we
change the coinbase additional input script data we can send a new job
dowsntream without worrying of invalidating miner's shares. When the
pool receive a prev hash it immidiatly invalidate all the previous jobs,
we still want to refuse shares for stale jobs. The client can easly handle
this situation: when a pool refuse a share it should start a timer and if do
not receive a new prev hash (or already have) within n seconds it change
pool.
This commit fix 2 miner things:

When we calculate the coinbase_prefix (what we need to put in the
extended job) we need to account also for the coinbase input script
additional data that is part of the extranonce.

When we create pool channel facotry we pass an extranonce creator and an
pool signature. If the signature + extranonce are bigger then 32 bytes
we have to return an error. That cause in sv2 the extranonce can not be
longer than 32 bytes.
The translator normalize the coinbase and remove the segwit data
from the coinbase prefix and suffix. In order to do that it need to know
the extranonce len, we used a default value of 322 bytes, but the pool
could use also smalle extranonces.
 Add a method that the pool can use in order to change
 coinbase_script_additional_data of an already opened
 channel and send downstream the new extranonce_prefix
Add time buffer to fix signature verification error
Coinbase signature is not part of the Sv2 protocol, some pool maybe want
to use it other not. The first part of the extranonce could also be
reserved for things that are not a pool signature. This pr rename the
pool_signature field of the channel factory into additional_coinbase_script_data
and change the type from Strgin to Vec<u8>, since can be anything.
The coinbase input script additional data should be sent as part of the
extranonce_prefix and not as part of the coinbase_prefix. So that a JDC
can see what the pool want as coinbase input script additional data
without the need to observ the coinbase prefix in job constructed by the
pool.
Update the pool to use an extranonce of 16 bytes rather then 32 so that
there is enaugh space to add the additional coinbase input script data.
Right now the channel factory only support one active job at time. That
means that if we receive a share for a job right after we sent downstream
a new job that share will be invalid. Now the channel factory keep track
of the last 3 jobs, so we give time to the dowstream to receive the job
and propagate it down before stop accepting shares for older job. This
is useful, and the system can be more responsive: as soon as we
change the coinbase additional input script data we can send a new job
dowsntream without worrying of invalidating miner's shares. When the
pool receive a prev hash it immidiatly invalidate all the previous jobs,
we still want to refuse shares for stale jobs. The client can easly handle
this situation: when a pool refuse a share it should start a timer and if do
not receive a new prev hash (or already have) within n seconds it change
pool.
This commit fix 2 miner things:

When we calculate the coinbase_prefix (what we need to put in the
extended job) we need to account also for the coinbase input script
additional data that is part of the extranonce.

When we create pool channel facotry we pass an extranonce creator and an
pool signature. If the signature + extranonce are bigger then 32 bytes
we have to return an error. That cause in sv2 the extranonce can not be
longer than 32 bytes.
The translator normalize the coinbase and remove the segwit data
from the coinbase prefix and suffix. In order to do that it need to know
the extranonce len, we used a default value of 322 bytes, but the pool
could use also smalle extranonces.
 Add a method that the pool can use in order to change
 coinbase_script_additional_data of an already opened
 channel and send downstream the new extranonce_prefix
fi3 added 18 commits December 19, 2024 09:44
Coinbase signature is not part of the Sv2 protocol, some pool maybe want
to use it other not. The first part of the extranonce could also be
reserved for things that are not a pool signature. This pr rename the
pool_signature field of the channel factory into additional_coinbase_script_data
and change the type from Strgin to Vec<u8>, since can be anything.
The coinbase input script additional data should be sent as part of the
extranonce_prefix and not as part of the coinbase_prefix. So that a JDC
can see what the pool want as coinbase input script additional data
without the need to observ the coinbase prefix in job constructed by the
pool.
Update the pool to use an extranonce of 16 bytes rather then 32 so that
there is enaugh space to add the additional coinbase input script data.
Right now the channel factory only support one active job at time. That
means that if we receive a share for a job right after we sent downstream
a new job that share will be invalid. Now the channel factory keep track
of the last 3 jobs, so we give time to the dowstream to receive the job
and propagate it down before stop accepting shares for older job. This
is useful, and the system can be more responsive: as soon as we
change the coinbase additional input script data we can send a new job
dowsntream without worrying of invalidating miner's shares. When the
pool receive a prev hash it immidiatly invalidate all the previous jobs,
we still want to refuse shares for stale jobs. The client can easly handle
this situation: when a pool refuse a share it should start a timer and if do
not receive a new prev hash (or already have) within n seconds it change
pool.
This commit fix 2 miner things:

When we calculate the coinbase_prefix (what we need to put in the
extended job) we need to account also for the coinbase input script
additional data that is part of the extranonce.

When we create pool channel facotry we pass an extranonce creator and an
pool signature. If the signature + extranonce are bigger then 32 bytes
we have to return an error. That cause in sv2 the extranonce can not be
longer than 32 bytes.
The translator normalize the coinbase and remove the segwit data
from the coinbase prefix and suffix. In order to do that it need to know
the extranonce len, we used a default value of 322 bytes, but the pool
could use also smalle extranonces.
 Add a method that the pool can use in order to change
 coinbase_script_additional_data of an already opened
 channel and send downstream the new extranonce_prefix
@Fi3 Fi3 changed the title [WIP] Remove short ids Remove short ids Dec 20, 2024
@plebhash plebhash mentioned this pull request Jan 13, 2025
@GitGab19
Copy link
Collaborator

GitGab19 commented Feb 3, 2025

As agreed with @Fi3, we are going to adapt this PR to current main.

This will be part of roles_logic_sv2 refactoring.

Opened an issue for it: #1440.

Moving this PR to draft so that it can still be used by @plebhash while working on #1440.

@GitGab19 GitGab19 marked this pull request as draft February 3, 2025 11:10
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