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

feat: mnlistdiff move nversion to first position #5450

Merged

Conversation

ogabrielides
Copy link
Collaborator

@ogabrielides ogabrielides commented Jun 21, 2023

Issue being fixed or feature implemented

Version field should always be the first field of a message for better readibility.

What was done?

  • Introduced new protocol version MNLISTDIFF_VERSION_ORDER (70229).
  • nVersion serialisation order is changed for clients with protocol version greater than or equal to 70229.
  • For clients with protocol version >= 70225 and < 70229 the old order is used: can be deprecated in the future.
  • Increased functional test P2P mininode's protocol version to 70229.

How Has This Been Tested?

feature_llmq_rotation.py with new protocol version.

Breaking Changes

cc @HashEngineering

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)

@ogabrielides ogabrielides added this to the 20 milestone Jun 21, 2023
@ogabrielides ogabrielides added the P2P Some notable changes on p2p level label Jun 21, 2023
@PastaPastaPasta
Copy link
Member

light-NACK; Yes the version should be first but... do we really need to change it? I'd prefer to just avoid the complexity and added proto version unless mobile will have a hard time without this change (which looking at it, maybe they will/would, but I want that clarified)

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 It's not necessary atm, true. But it's trivial and mnlistdiff is going to be changed in #5377/v20 anyway so why not clean things up while we are at it?

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; okay

@PastaPastaPasta PastaPastaPasta merged commit 0e53540 into dashpay:develop Jun 26, 2023
@ogabrielides ogabrielides deleted the mnlistdiff_version_order branch June 26, 2023 07:47
thephez added a commit to thephez/docs-core that referenced this pull request Aug 16, 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 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2P Some notable changes on p2p level
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants