feat: auto generation EHF and spork+EHF activation for MN_RR #5597

knst · 2023-10-01T19:53:58Z

Implementation EHF mechanism, part 4. Previous changes are:

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:

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 (for repository code-owners and collaborators only)

github-actions · 2023-10-03T14:35:17Z

This pull request has conflicts, please rebase.

github-actions · 2023-10-06T16:04:48Z

This pull request has conflicts, please rebase.

github-actions · 2023-10-09T14:19:44Z

This pull request has conflicts, please rebase.

UdjinM6

pls see 2ccf9d1

knst · 2023-10-09T21:08:56Z

@UdjinM6
A.
2ccf9d1#diff-4a04bc0b355c780033960e8c261ee9b6d3c452897e1dcd88a15d272512266c76R1133

self.activate_mn_rr(expected_activation_height=node.getblockcount() + 12 * 3)

but it can take a time until recovered sigs are ready and tx with EHF signal is submitted more than one loop iteration:

       while mn_rr_status == 0:
            time.sleep(1)
            mn_rr_status = get_bip9_details(self.nodes[0], 'mn_rr')['ehf']
            self.nodes[0].generate(1)

So, if sig for EHF signal is delayed more than 1 second and 1 block -> it may be more that 12 * 3 blocks required to activate mn_rr, isn't it?

B.
consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nWindowSize = 4032;
When v20 would be released it will take time until v20 would be activated (several weeks, 4-6?). After that after spork activation it would be required to wait 4-6 weeks more. Is it expected to wait 4-6 weeks more?

UdjinM6 · 2023-10-10T10:07:16Z

@UdjinM6 A. 2ccf9d1#diff-4a04bc0b355c780033960e8c261ee9b6d3c452897e1dcd88a15d272512266c76R1133

self.activate_mn_rr(expected_activation_height=node.getblockcount() + 12 * 3)

but it can take a time until recovered sigs are ready and tx with EHF signal is submitted more than one loop iteration:
       while mn_rr_status == 0:
            time.sleep(1)
            mn_rr_status = get_bip9_details(self.nodes[0], 'mn_rr')['ehf']
            self.nodes[0].generate(1)
So, if sig for EHF signal is delayed more than 1 second and 1 block -> it may be more that 12 * 3 blocks required to activate mn_rr, isn't it?

bip9 works in periods. Also, in this specific case we start activating at the beginning of the period (block 2796) and we have threshold which is lower than that so missing a couple of blocks is not a problem. But I agree that calculating it like that is probably misleading. We could simply go with a static number instead I guess i.e. expected_activation_height=2832.

B. consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nWindowSize = 4032; When v20 would be released it will take time until v20 would be activated (several weeks, 4-6?). After that after spork activation it would be required to wait 4-6 weeks more. Is it expected to wait 4-6 weeks more?

Good point 👍 Pls ignore this part of my suggestions then :)

UdjinM6

re-utACK

src/llmq/ehf_signals.h

UdjinM6

LGTM, utACK

PastaPastaPasta

light-ACK for squash merge

github-actions · 2023-10-17T20:52:39Z

This pull request has conflicts, please rebase.

…Mempool This commit adds basic implementation for auto-generation of EHF signals. This implementation is a basement for various EHF (include MN_RR). Please, notice that some unit and functional tests are broken after this commit due to unexpected activation of MN_RR (or in opposited expected non-activation) Next commit is adding a new Spork SPORK_24_MN_RR_READY and fixes all Unit and Functional tests accordingly new logic of MN_RR activation

This spork is used to signalize EhfSignalsManager that EHF signal for fork MN_RR can be signed and send to network now (platform is ready)

Co-authored-by: PastaPastaPasta <[email protected]>

…Handler constructor

knst · 2023-10-17T21:26:30Z

@knst knst force-pushed the mnehf-manager branch from b6b165f to 7e96c96

due to

This pull request has conflicts, please rebase.

test/functional/feature_asset_locks.py

UdjinM6

utACK

PastaPastaPasta

re-utACK

## Issue being fixed or feature implemented #5597 follow-up ## What was done? add missing filed description ## How Has This Been Tested? `help getblockchaininfo` ## Breaking Changes n/a ## Checklist: - [x] 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 - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

knst marked this pull request as draft October 1, 2023 19:54

knst added this to the 20 milestone Oct 2, 2023

github-actions bot added the needs rebase label Oct 3, 2023

knst force-pushed the mnehf-manager branch from c199862 to dc8b298 Compare October 5, 2023 18:02

github-actions bot removed the needs rebase label Oct 5, 2023

knst force-pushed the mnehf-manager branch 2 times, most recently from efe7520 to 8640df5 Compare October 6, 2023 08:55

knst added RPC Some notable changes to RPC params/behaviour/descriptions Needs Release Notes This PR includes breaking changes for which release notes have not yet been merged. labels Oct 6, 2023

github-actions bot added the needs rebase label Oct 6, 2023

knst force-pushed the mnehf-manager branch from 8640df5 to 76bbc82 Compare October 6, 2023 16:09

github-actions bot removed the needs rebase label Oct 6, 2023

knst force-pushed the mnehf-manager branch from 76bbc82 to 05c7285 Compare October 9, 2023 10:51

knst requested review from UdjinM6, ogabrielides and PastaPastaPasta October 9, 2023 10:59

knst marked this pull request as ready for review October 9, 2023 11:06

knst changed the title ~~feat: auto generation EHF~~ feat: auto generation EHF and MN_RR spork+EHF activation Oct 9, 2023

knst changed the title ~~feat: auto generation EHF and MN_RR spork+EHF activation~~ feat: auto generation EHF and spork+EHF activation for MN_RR Oct 9, 2023

github-actions bot added the needs rebase label Oct 9, 2023

knst force-pushed the mnehf-manager branch from 05c7285 to 1cafa48 Compare October 9, 2023 14:59

github-actions bot removed the needs rebase label Oct 9, 2023

knst force-pushed the mnehf-manager branch from 1cafa48 to 654fa9d Compare October 9, 2023 20:58

UdjinM6 requested changes Oct 9, 2023

View reviewed changes

UdjinM6 force-pushed the mnehf-manager branch from a92e51e to 0c5f61e Compare October 12, 2023 16:14

knst dismissed UdjinM6’s stale review via 9e9ed91 October 17, 2023 13:34

UdjinM6 previously approved these changes Oct 17, 2023

View reviewed changes

PastaPastaPasta reviewed Oct 17, 2023

View reviewed changes

src/llmq/ehf_signals.h Outdated Show resolved Hide resolved

knst dismissed UdjinM6’s stale review via b6b165f October 17, 2023 17:58

knst requested a review from PastaPastaPasta October 17, 2023 18:17

UdjinM6 previously approved these changes Oct 17, 2023

View reviewed changes

PastaPastaPasta previously approved these changes Oct 17, 2023

View reviewed changes

github-actions bot added the needs rebase label Oct 17, 2023

knst and others added 10 commits October 18, 2023 03:56

feat: provide information about MN EHF forks in RPC blockchaininfo

1c97916

fix: validation of IsTxSafeForMining for txes without input

d5d691f

feat: new spork SPORK_24_MN_RR_READY and related logic

7b942f7

This spork is used to signalize EhfSignalsManager that EHF signal for fork MN_RR can be signed and send to network now (platform is ready)

fix: adds a mutex to avoid using ids in ehf_signals without locks

2ca70f1

fix: missing LOCKS_EXCLUDED

f42fb83

suggestions

f871869

fix: add TODO about window size. 2016 to setup maybe?

bc68f4d

fix(trivial): comment src/evo/mnhftx.h

34c66ff

Co-authored-by: PastaPastaPasta <[email protected]>

fix: added missing const and removed non-used argument in CEHFSignals…

7e96c96

…Handler constructor

knst dismissed stale reviews from PastaPastaPasta and UdjinM6 via 7e96c96 October 17, 2023 21:26

knst force-pushed the mnehf-manager branch from b6b165f to 7e96c96 Compare October 17, 2023 21:26

github-actions bot removed the needs rebase label Oct 17, 2023

UdjinM6 reviewed Oct 17, 2023

View reviewed changes

test/functional/feature_asset_locks.py Show resolved Hide resolved

UdjinM6 approved these changes Oct 17, 2023

View reviewed changes

PastaPastaPasta approved these changes Oct 18, 2023

View reviewed changes

PastaPastaPasta merged commit 63ed462 into dashpay:develop Oct 18, 2023
10 of 11 checks passed

UdjinM6 mentioned this pull request Oct 20, 2023

fix: add rpc help text for "ehf" field #5628

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: auto generation EHF and spork+EHF activation for MN_RR #5597

feat: auto generation EHF and spork+EHF activation for MN_RR #5597

knst commented Oct 1, 2023 •

edited

Loading

github-actions bot commented Oct 3, 2023

github-actions bot commented Oct 6, 2023

github-actions bot commented Oct 9, 2023

UdjinM6 left a comment

knst commented Oct 9, 2023

UdjinM6 commented Oct 10, 2023

UdjinM6 left a comment

UdjinM6 left a comment

PastaPastaPasta left a comment

github-actions bot commented Oct 17, 2023

knst commented Oct 17, 2023

UdjinM6 left a comment

PastaPastaPasta left a comment

feat: auto generation EHF and spork+EHF activation for MN_RR #5597

feat: auto generation EHF and spork+EHF activation for MN_RR #5597

Conversation

knst commented Oct 1, 2023 • edited Loading

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

github-actions bot commented Oct 3, 2023

github-actions bot commented Oct 6, 2023

github-actions bot commented Oct 9, 2023

UdjinM6 left a comment

Choose a reason for hiding this comment

knst commented Oct 9, 2023

UdjinM6 commented Oct 10, 2023

UdjinM6 left a comment

Choose a reason for hiding this comment

UdjinM6 left a comment

Choose a reason for hiding this comment

PastaPastaPasta left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 17, 2023

knst commented Oct 17, 2023

UdjinM6 left a comment

Choose a reason for hiding this comment

PastaPastaPasta left a comment

Choose a reason for hiding this comment

knst commented Oct 1, 2023 •

edited

Loading