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.

Sign up for GitHub

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

Jump to bottom

fix: Stop smeshing clean up smeshing-proving-opts in config #1392

Closed
wants to merge 7 commits into from
Closed

fix: Stop smeshing clean up smeshing-proving-opts in config #1392

wants to merge 7 commits into from

Conversation

maparr
Copy link
Contributor

@maparr maparr commented Jul 21, 2023 •
edited
Loading

It closes #1385

How to test:

Screenshot 2023-07-21 at 17 36 57

A: missing smeshing-proving-opts

  1. open custom config and remove smeshing-proving-opts ( init the smeshing before )
  2. sign in
  3. see the modal
  4. update settings
  5. settings should be updated

B: update smeshing-proving-opts

  1. start smashing
  2. click edit
  3. edit PoS proving opts
  4. see the smeshing update
  5. settings should be updated

C: update smeshing-proving-opts with enabled auto-start

  1. enable auto-start
  2. restart PC
  3. see the opened app and the modal with update proving settings
  4. update settings
  5. settings should be updated

D: update smeshing-proving-opts with the broken node by default ( for example remove the boot nodes from the config )

  1. start the app
  2. node is down
  3. see modal with update proving settings
  4. update settings
  5. settings should be updated
Screenshot 2023-07-21 at 16 59 02 Screenshot 2023-07-21 at 16 59 13 Screenshot 2023-07-21 at 17 05 25 Screenshot 2023-07-21 at 16 59 25 Screenshot 2023-07-21 at 17 43 48

Checklist

need to test and resolve additional cases from the code

  • wallet only
  • auto-start

@maparr maparr self-assigned this Jul 21, 2023
@github-actions
Copy link

Compiled Binaries

@maparr maparr marked this pull request as ready for review July 21, 2023 16:28
@github-actions
Copy link

Compiled Binaries

@github-actions
Copy link

Compiled Binaries

@monikasmolarek
Copy link
Collaborator

monikasmolarek commented Jul 21, 2023 •
edited
Loading

Changing the pos proving opts while the POS generation is paused does not save the update in the configs, even though the change appears on the smeshing screen. Even app restart doesn’t update the configs. Is it meant to be updated only while the smeshing process keeps running/is resumed?

Screen.Recording.2023-07-21.at.22.22.mov

@maparr
Copy link
Contributor Author

maparr commented Jul 22, 2023

Changing the pos proving opts while the POS generation is paused does not save the update in the configs, even though the change appears on the smeshing screen. Even app restart doesn’t update the configs. Is it meant to be updated only while the smeshing process keeps running/is resumed?

Screen.Recording.2023-07-21.at.22.22.mov

after the update, Im writhing to node config, and after it do stop / start for smeshing process. The Idea was to reuse our code, because we didn't have functionality to update config from client side

@github-actions
Copy link

Compiled Binaries

@github-actions
Copy link

Compiled Binaries

@github-actions
Copy link

Compiled Binaries

@github-actions
Copy link

Compiled Binaries

@monikasmolarek
Copy link
Collaborator

There was an announcement asking the users to update manually their proving opts.
In case someone chose incorrect values - do we want to handle the check and display the modal or leave the users make their mistakes?
Screenshot 2023-07-22 at 14 42 59

@monikasmolarek
Copy link
Collaborator

Also, I think there's no question about this case, the modal should pop up, now it doesn;t show up:

one of the proving opts set to 0
Screenshot 2023-07-22 at 14 45 10
or one of the proving opts missing:
Screenshot 2023-07-22 at 14 53 36

@github-actions
Copy link

Compiled Binaries

@github-actions
Copy link

Compiled Binaries

@monikasmolarek
Copy link
Collaborator

@maparr will this one be amended in terms of the smashing opts set to 0 (as discussed on Saturday evening)?

@maparr maparr marked this pull request as draft July 24, 2023 14:26
@maparr maparr closed this Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brusherru brusherru Awaiting requested review from brusherru

@pigmej pigmej Awaiting requested review from pigmej

@monikasmolarek monikasmolarek Awaiting requested review from monikasmolarek

Assignees

@maparr maparr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Stop smeshing clean up smeshing-proving-opts in config
2 participants
@maparr @monikasmolarek