Skip to content

Conversation

@knst
Copy link
Collaborator

@knst knst commented Sep 22, 2024

Issue being fixed or feature implemented

DashTestFramework requires 2 arguments for dashd which are lost every dashd restart: sporkkey and dip3params.
Without this PR you need to pass this arguments manually every time when you restart dashd in functional tests.

What was done?

Make dash specific args sporkkey and dip3params resilient for dashd restart

It makes workarounds such as these no more needed:

self.restart_node(1, self.extra_args[1] + ["-checkaddrman=1"])

Also there's some cleanup for wallet_mnemonicbits.py to remove usehd=1 which is not required, and it has been discovered during revising all node's restarts related code.

How Has This Been Tested?

Removed workaround from rpc_net.py and run functional tests

Breaking Changes

N/A

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone

@knst knst added this to the 21.2 milestone Sep 22, 2024
@github-actions
Copy link

This pull request has conflicts, please rebase.

self.extra_args[0] += ["-sporkkey=cP4EKFyJsHT39LDqgdcB43Y3YXjNyjb5Fuas1GQSeAtjnZWmZEQK"]
for i in range(0, num_nodes):
self.extra_args[i].append("-dip3params=2:2")
self.extra_args_from_options += ["-sporkkey=cP4EKFyJsHT39LDqgdcB43Y3YXjNyjb5Fuas1GQSeAtjnZWmZEQK", "-dip3params=2:2"]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6a161ab: sporkkey should be set on node0 only. Also, we probably should be using dash.conf for options that are meant to be persistent, pls check 3237354.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why exactly sporkkey can't be specified for all nodes? It's out of scope for all tests except "feature_sporks.py" to care about sporks functionally

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do that. But you shouldn't :D

knst and others added 2 commits October 11, 2024 16:38
…nt for dashd restart

Co-Authored-By: UdjinM6 <UdjinM6@users.noreply.github.com>
@knst knst force-pushed the fix-reuse-extra-args branch from 8b3c83e to 6d4a782 Compare October 11, 2024 09:41
@knst knst marked this pull request as ready for review October 11, 2024 09:41
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 6d4a782

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 6d4a782

@PastaPastaPasta PastaPastaPasta merged commit dc02da1 into dashpay:develop Oct 15, 2024
32 of 35 checks passed
@knst knst deleted the fix-reuse-extra-args branch October 15, 2024 14:07
@UdjinM6 UdjinM6 modified the milestones: 21.2, 22 Oct 29, 2024
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