Skip to content

Conversation

@UdjinM6
Copy link

@UdjinM6 UdjinM6 commented May 25, 2023

Issue being fixed or feature implemented

What was done?

pls see individual commits

How Has This Been Tested?

reorg mainnet around forkpoint with a patched client (to allow low difficulty), run tests

Breaking Changes

Another evodb migration is required. Going back to an older version or migrating after the fork requires reindexing.

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)

Copy link

@ogabrielides ogabrielides left a comment

Choose a reason for hiding this comment

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

Copy link

@ogabrielides ogabrielides left a comment

Choose a reason for hiding this comment

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

@UdjinM6
Copy link
Author

UdjinM6 commented May 26, 2023

I am confused.. why here we pass basic BLS ?

we don't

void AddMN(const CDeterministicMNCPtr& dmn, bool fBumpTotalCount = true);

@ogabrielides
Copy link

@UdjinM6
Copy link
Author

UdjinM6 commented May 28, 2023

implemented similar changes, fixed a couple of issues in migration logic and squashed 23770b525affa9644526a5b2f79762eabd09a0d5 and all these fixes into original commits

@UdjinM6
Copy link
Author

UdjinM6 commented May 31, 2023

superseded by #5403

@UdjinM6 UdjinM6 closed this May 31, 2023
UdjinM6 added a commit that referenced this pull request Jun 4, 2023
## Issue being fixed or feature implemented
same as  #5392, alternative solution

~based on #5402 atm, will rebase later~

## What was done?
pls see individual commits

## How Has This Been Tested?
reorg mainnet around forkpoint with a patched client (to allow low
difficulty), run tests

## Breaking Changes
Another evodb migration is required. Going back to an older version or
migrating after the fork requires reindexing.

## Checklist:
- [x] I have performed a self-review of my own code
- [x] 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)_
PastaPastaPasta pushed a commit that referenced this pull request Jun 11, 2023
…/deser pubKeyOperator (#5397)

## Issue being fixed or feature implemented
Mobile wallets would have to convert 4k+ pubkeys at the V19 fork point
and it's a pretty hard job for them that can easily take 10-15 seconds
if not more. Also after the HF, if a masternode list is requested from
before the HF, the operator keys come in basic scheme, but the
merkelroot was calculated with legacy. From mobile team work it wasn't
possible to convert all operator keys to legacy and then calculate the
correct merkleroot.

~This PR builds on top of ~#5392~ #5403 (changes that belong to this PR:
26f7e966500bdea4c604f1d16716b40b366fc707 and
4b42dc8fcee3354afd82ce7e3a72ebe1659f5f22) and aims to solve both of
these issues.~

cc @HashEngineering @QuantumExplorer 

## What was done?
Introduce `nVersion` on p2p level for every CSimplifiedMNListEntry. Set
`nVersion` to the same value we have it in CDeterministicMNState i.e.
pubkey serialization would not be via basic scheme only after the V19
fork, it would match the way it’s serialized on-chain/in
CDeterministicMNState for that specific MN.

## How Has This Been Tested?
run tests

## Breaking Changes
NOTE: `testnet` is going to re-fork at v19 forkpoint because
`merkleRootMNList` is not going to match

## 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
- [ ] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 11, 2023
same as  dashpay#5392, alternative solution

~based on dashpay#5402 atm, will rebase later~

pls see individual commits

reorg mainnet around forkpoint with a patched client (to allow low
difficulty), run tests

Another evodb migration is required. Going back to an older version or
migrating after the fork requires reindexing.

- [x] I have performed a self-review of my own code
- [x] 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)_
UdjinM6 added a commit to UdjinM6/dash that referenced this pull request Jun 11, 2023
…/deser pubKeyOperator (dashpay#5397)

Mobile wallets would have to convert 4k+ pubkeys at the V19 fork point
and it's a pretty hard job for them that can easily take 10-15 seconds
if not more. Also after the HF, if a masternode list is requested from
before the HF, the operator keys come in basic scheme, but the
merkelroot was calculated with legacy. From mobile team work it wasn't
possible to convert all operator keys to legacy and then calculate the
correct merkleroot.

~This PR builds on top of ~dashpay#5392~ dashpay#5403 (changes that belong to this PR:
26f7e966500bdea4c604f1d16716b40b366fc707 and
4b42dc8fcee3354afd82ce7e3a72ebe1659f5f22) and aims to solve both of
these issues.~

cc @HashEngineering @QuantumExplorer

Introduce `nVersion` on p2p level for every CSimplifiedMNListEntry. Set
`nVersion` to the same value we have it in CDeterministicMNState i.e.
pubkey serialization would not be via basic scheme only after the V19
fork, it would match the way it’s serialized on-chain/in
CDeterministicMNState for that specific MN.

run tests

NOTE: `testnet` is going to re-fork at v19 forkpoint because
`merkleRootMNList` is not going to match

- [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
- [ ] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 12, 2023
…/deser pubKeyOperator (dashpay#5397)

Mobile wallets would have to convert 4k+ pubkeys at the V19 fork point
and it's a pretty hard job for them that can easily take 10-15 seconds
if not more. Also after the HF, if a masternode list is requested from
before the HF, the operator keys come in basic scheme, but the
merkelroot was calculated with legacy. From mobile team work it wasn't
possible to convert all operator keys to legacy and then calculate the
correct merkleroot.

~This PR builds on top of ~dashpay#5392~ dashpay#5403 (changes that belong to this PR:
26f7e966500bdea4c604f1d16716b40b366fc707 and
4b42dc8fcee3354afd82ce7e3a72ebe1659f5f22) and aims to solve both of
these issues.~

cc @HashEngineering @QuantumExplorer

Introduce `nVersion` on p2p level for every CSimplifiedMNListEntry. Set
`nVersion` to the same value we have it in CDeterministicMNState i.e.
pubkey serialization would not be via basic scheme only after the V19
fork, it would match the way it’s serialized on-chain/in
CDeterministicMNState for that specific MN.

run tests

NOTE: `testnet` is going to re-fork at v19 forkpoint because
`merkleRootMNList` is not going to match

- [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
- [ ] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
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.

2 participants