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

fix!: mn_rr features only for v21+ #5642

Merged
merged 3 commits into from
Oct 23, 2023
Merged

Conversation

knst
Copy link
Collaborator

@knst knst commented Oct 23, 2023

Issue being fixed or feature implemented

Should not be 2 forks in one version

What was done?

  • Asset Unlock transactions (withdrawals) should be available only in MN_RR fork
  • MN_RR should not be auto-activated on Main net without intentional release of code (and not by spork), but they are need on test net to test platform.

How Has This Been Tested?

Run unit/functional tests

Breaking Changes

Yes (see "what was done")

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 20 milestone Oct 23, 2023
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 for squash merge; this looks correct to me

@UdjinM6
Copy link

UdjinM6 commented Oct 23, 2023

MN_RR should not be auto-activated on Main net without intentional release of code (and not by spork), but they are need on test net to test platform

so is SPORK_24_EHF not longer need or is it a non-mainnet only spork now?

@PastaPastaPasta
Copy link
Member

MN_RR should not be auto-activated on Main net without intentional release of code (and not by spork), but they are need on test net to test platform

so is SPORK_24_EHF not longer need or is it a non-mainnet only spork now?

Basically non-main net only, a value could still be set; but it doesn't (shouldn't) affect anything on mainnet

src/evo/specialtxman.cpp Outdated Show resolved Hide resolved
@knst
Copy link
Collaborator Author

knst commented Oct 23, 2023

so is SPORK_24_EHF not longer need or is it a non-mainnet only spork now?

This spork has been introduced to prevent activation of mn_rr before platform is ready.

As discussed today:

  • Asset Unlock transaction is not tested properly on testnet yet; not tested properly feature should not be potentially active on mainnet; also it may require consensus rules to chage
  • 2 forks are confusing for dash users. Let's keep the rule "1 major version = 1 hard-fork"
  • activation of a hard fork by spork is it bad idea; because it should relay on user's update, not DCG command to do anything.

So, mn_rr requires a new release v21 or v20.1; the spork maybe even dropped then.

For testnet it is needed for platform's need + to test EHF mechanism.

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

@PastaPastaPasta
Copy link
Member

Issue created here: #5644

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 for squash merge

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