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

Ensure saved offchain solution matches creation snapshop #462

Open
Tracked by #461
kianenigma opened this issue Apr 29, 2021 · 6 comments
Open
Tracked by #461

Ensure saved offchain solution matches creation snapshop #462

kianenigma opened this issue Apr 29, 2021 · 6 comments
Labels
I2-bug The node fails to follow expected behavior.

Comments

@kianenigma
Copy link
Contributor

Related to paritytech/substrate#8641 and paritytech/substrate#8290.

We will support offchain worker resubmission soon. The problem is that the solution is created once and then sent blindly again. This can lead to bad solutions because the snapshot might have changed due to a revert.

Initially, we thought we can use fork-aware offchain storage, but that is not implemented.

Alternatively, in paritytech/substrate#8290 we fall back to just calling feasibility_check one last time before resubmission of cached solution to makes sure it is not faffy.

This is rather suboptimal. A nicer approach is to have a hash of the snapshot saved somewhere alongside the cached solution, so we can check it against the hash that is stored onchain.

@stale
Copy link

stale bot commented Jul 7, 2021

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale label Jul 7, 2021
@kianenigma
Copy link
Contributor Author

Quite important. The OCW miners could repeatedly come to an wrong solution because of this.

@kianenigma
Copy link
Contributor Author

This will actually not solve anything for the unsigned phase, because as long as you are in the same thread, you will surely read the same hash since you are in the same OCW thread.

note that if there is a long enough signed phase, this is not a big deal for the unsigned phase.

note that a good easy solution to all of this is to put a gap between snapshot and any submission.

lastly, we should have a safeguard in place, to make sure if for any reason OCW submission of any validator fails, then they do not retry in the same session.

@kianenigma
Copy link
Contributor Author

This is still a nice addition, but very low quality, as doing a feasibility check before submitting offchain is not a big deal.

@kianenigma
Copy link
Contributor Author

I checked this again. Indeed, we need to remember that within an OCW thread, the snapshot hash will always match, because even if we progress 10 blocks, the thread is blind to the progress. But:

  1. When we go over the pool, in vlaidate_transaction, we could utilize this.
  2. When we resubmit cached offchain solutions, we could utilize this.

This is a good, medium difficulty, low priority task. @Ank4n @gpestana either of you can pick it up as a side task.

@Ank4n
Copy link
Contributor

Ank4n commented Nov 24, 2022

I would like to attempt this and potentially pair with @gpestana if you are up for it?

@juangirini juangirini transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added I2-bug The node fails to follow expected behavior. and removed I3-bug labels Aug 25, 2023
claravanstaden pushed a commit to Snowfork/polkadot-sdk that referenced this issue Dec 8, 2023
Co-authored-by: Philip Stanislaus <[email protected]>
Co-authored-by: Aidan Musnitzky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I2-bug The node fails to follow expected behavior.
Projects
Status: ⌛️ Sometime-soon
Development

No branches or pull requests

3 participants