Skip to content

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

@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