Open GitGab19 opened 1 month ago
Oh, a tiny detail/suggestion I missed.
In the case in which Pool is receiving a wrong SetCustomMiningJob
(for example containing an old prev_hash
) it should answer to JDC with a SetCustomMiningJob.Error
. Also in the specs, there's a suggested error-code called invalid-job-param-value-{}
which could be perfect for this: https://github.com/stratum-mining/sv2-spec/blob/main/05-Mining-Protocol.md#5320-setcustomminingjoberror-server---client
As I understood the specs, the DeclareMiningJobSuccess.new_mining_job_token
can be used to commit the declared job. This token has to be different then AllocateMiningJobTokenSuccess.mining_job_token
(otherwise the doenstream can start mining before receiving a SetCustomMiningJobSeccess
) and should contain the JDS signature, covering the declared job. When the jdc sends a setCustomMiningJob
, the pool just checks the signature against the JDS pubkey.
Can take this up
Pool should just verify if the token is valid:
Pool should just verify if the token is valid:
@Fi3 these are the messages exchanged between JDC and JDS during Job Declaration:
AllocateMiningJobToken
+ AllocateMiningJobToken.Success
DeclareMiningJob
+ DeclareMiningJob.Success
none of these messages contain any information about prev_hash
, so even though mining_job_token
is valid, it shouldn't be used as the only metric for the pool to accept SetCustomMiningJob
messages.
what if JDC sends a SetCustomMiningJob
with a valid mining_job_token
but an old prev_hash
? or worse: some prev_hash
that is nonsense?
that could be a bug or an attack from JDC... but either way, the pool is going to reward JDC for shares that are actually worthless
and that was the discussion I had with @GitGab19 , where we both arrived to the conclusion that a pool should only respond with a SetCustomMiningJob.Success
if SetCustomMiningJob
carries the correct prev_hash
(which represents the current state of the network).
IMO pool should not enforce the prev hash. Miners will chose on which chain to mine and be rewarded accordingly. This to lower the effectiveness of selfish mining.
IMO pool should not enforce the prev hash. Miners will chose on which chain to mine and be rewarded accordingly. This to lower the effectiveness of selfish mining.
Are you suggesting here that responsibility about checking the prev-hash is on JDS, right?
While I and @plebhash were working on #923, he made a good point, which I'm going to try to summarize here.
When JDC declares a mining job to JDS, theoretically JDS should check it, and approve/deny it. After that, JDC communicated to Pool the job it's going to be used through the
SetCustomMiningJob
message.But in our current state, Pool is not checking fields contained in that message, and so a malicious JDC could try to cheat the Pool by using a wrong field (such as the
prev_hash
). In addiction to that, while looking at Pool handlers, I found this function which calls thecheck_set_custom_mining_job
one, which always returnstrue
(so doing no checks at all).So we need to implement those checks on Pool side, in the places I linked in this description.
Please @plebhash feel free to expand if you think I missed something 🙏