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!: making MnEhfTx to comply DIP-0023 #5505

Merged
merged 7 commits into from
Jul 25, 2023
Merged

Conversation

knst
Copy link
Collaborator

@knst knst commented Jul 24, 2023

Issue being fixed or feature implemented

Current implementation of MnEhfTx is not matched with DIP-0023, this PR fixes it. It is a prior work for #5469

What was done?

  • requestID is fixed from clsig{quorumHeight} to mnhf{versionBit} + fixes for signature validation properly
  • v20 is minimal height to accept MnEHF special transactions
  • versionBit is not BLS version - removed unrelated wrong code and validations
  • TxMempool will accept MnEHF transaction even if inputs/outputs are zeroes and no fee
  • implemented python's serialization/deserialization of MnEHF transactions for future using in functional tests

How Has This Been Tested?

Run functional/unit tests. Beside that there's new functional test in #5469 that actually test format of transaction and signature validation - to be merged later.

Breaking Changes

Payload of MnEhf tx is changed, related consensus rules are changed.

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 requested review from UdjinM6 and PastaPastaPasta July 24, 2023 20:50
@UdjinM6 UdjinM6 added this to the 20 milestone Jul 24, 2023
src/evo/mnhftx.cpp Outdated Show resolved Hide resolved
src/evo/mnhftx.h Outdated Show resolved Hide resolved
@@ -53,14 +50,16 @@ class MNHFTxPayload
static constexpr auto SPECIALTX_TYPE = TRANSACTION_MNHF_SIGNAL;
static constexpr uint16_t CURRENT_VERSION = 1;

uint16_t nVersion{CURRENT_VERSION};
uint8_t nVersion{CURRENT_VERSION};
Copy link

Choose a reason for hiding this comment

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

uint16_t in DIP

Suggested change
uint8_t nVersion{CURRENT_VERSION};
uint16_t nVersion{CURRENT_VERSION};

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

src/evo/mnhftx.h Show resolved Hide resolved
@knst knst requested a review from UdjinM6 July 25, 2023 09:25
src/evo/mnhftx.cpp Outdated Show resolved Hide resolved
src/evo/mnhftx.h Outdated Show resolved Hide resolved
@knst knst requested a review from PastaPastaPasta July 25, 2023 15:30
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

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 to merge after dashpay/dips#138

@UdjinM6 UdjinM6 merged commit 42dcb3d into dashpay:develop Jul 25, 2023
thephez added a commit to thephez/docs-core that referenced this pull request Aug 16, 2023
thephez added a commit to thephez/docs-core that referenced this pull request Aug 30, 2023
thephez added a commit to dashpay/docs-core that referenced this pull request Aug 30, 2023
* docs: deprecate MSG_LEGACY_TXLOCK_REQUEST

Aligns with dashpay/dash#5483

* chore: update link to prev version of docs

* docs: update mnlistdiff nversion location

Relates to dashpay/dash#5450

* docs(p2p): update mnlistdiff

Relates to dashpay/dash#5377

* docs: update cbtx for v3

Relates to dashpay/dash#5262

* docs: update mnhf with details of final implementation

Relates to dashpay/dash#5469 and dashpay/dash#5505

* docs: note removal of NODE_GETUTXO

Relates to dashpay/dash#5500

* chore: revert "docs: update mnhf with details of final implementation"

This reverts commit 8e4bf6c since there
may still be additional changes to the implementation (it's not merged)
thephez added a commit to thephez/docs-core that referenced this pull request Sep 27, 2023
* docs: deprecate MSG_LEGACY_TXLOCK_REQUEST

Aligns with dashpay/dash#5483

* chore: update link to prev version of docs

* docs: update mnlistdiff nversion location

Relates to dashpay/dash#5450

* docs(p2p): update mnlistdiff

Relates to dashpay/dash#5377

* docs: update cbtx for v3

Relates to dashpay/dash#5262

* docs: update mnhf with details of final implementation

Relates to dashpay/dash#5469 and dashpay/dash#5505

* docs: note removal of NODE_GETUTXO

Relates to dashpay/dash#5500

* chore: revert "docs: update mnhf with details of final implementation"

This reverts commit 8e4bf6c since there
may still be additional changes to the implementation (it's not merged)
thephez added a commit to thephez/docs-core that referenced this pull request Oct 4, 2023
PastaPastaPasta added a commit that referenced this pull request Oct 18, 2023
Implementation EHF mechanism, part 4. Previous changes are: 
 - #4577
 - #5505
 - #5469

## Issue being fixed or feature implemented
Currently MN_RR is activated automatically by soft-fork activation after
v20 is activated.
It is not flexible enough, because platform may not be released by that
time yet or in opposite it can be too long to wait.
Also, any signal of EHF requires manual actions from MN owners to sign
EHF signal - it is automated here.

## What was done?
New spork `SPORK_24_MN_RR_READY`; new EHF manager that sign EHF signals
semi-automatically without manual actions; and send transaction with EHF
signal when signal is signed to network.
Updated rpc `getblockchaininfo` to return information about of EHF
activated forks.
Fixed function `IsTxSafeForMining` in chainlock's handler to skip
transactions without inputs (empty `vin`).

## How Has This Been Tested?
Run unit/functional tests. Some tests have been updated due to new way
of MN_RR activation: `feature_asset_locks.py`, `feature_mnehf.py`,
`feature_llmq_evo.py` and unit test `block_reward_reallocation_tests`.


## Breaking Changes
New way of MN_RR activation.

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
thephez added a commit to dashpay/docs-core that referenced this pull request Nov 13, 2023
* docs: update mnhf with details of final implementation

Relates to dashpay/dash#5469 and dashpay/dash#5505

* doc: misc updates and corrections

* docs: typo fix
thephez added a commit to dashpay/docs-core that referenced this pull request Nov 15, 2023
* docs: deprecate MSG_LEGACY_TXLOCK_REQUEST

Aligns with dashpay/dash#5483

* chore: update link to prev version of docs

* docs: update mnlistdiff nversion location

Relates to dashpay/dash#5450

* docs(p2p): update mnlistdiff

Relates to dashpay/dash#5377

* docs: update cbtx for v3

Relates to dashpay/dash#5262

* docs: update mnhf with details of final implementation

Relates to dashpay/dash#5469 and dashpay/dash#5505

* docs: note removal of NODE_GETUTXO

Relates to dashpay/dash#5500

* chore: revert "docs: update mnhf with details of final implementation"

This reverts commit 8e4bf6c since there
may still be additional changes to the implementation (it's not merged)
thephez added a commit to dashpay/docs-core that referenced this pull request Nov 15, 2023
* docs: update mnhf with details of final implementation

Relates to dashpay/dash#5469 and dashpay/dash#5505

* doc: misc updates and corrections

* docs: typo fix
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