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

multi: Prepare for DCP0007. #2679

Merged
merged 7 commits into from
Aug 13, 2021
Merged

multi: Prepare for DCP0007. #2679

merged 7 commits into from
Aug 13, 2021

Conversation

matheusd
Copy link
Member

This PR prepares the codebase for the DCP0007 update by introducing the agenda and performing some refactorings to make the actual implementation of the consensus change clearer.

This PR is NOT intended to produce any consensus-breaking changes. This was split off from the main work towards implementing DCP0007 so that the actual consensus change that will be introduced by a future PR is more easily reviewable by other developers.

Summary of changes:

  • Introduce DCP0007 related deployment and agenda
  • Add functions to determine the result of the agenda vote
  • Various test refactorings

Copy link
Member

@rstaudt2 rstaudt2 left a comment

Choose a reason for hiding this comment

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

This looks good to me, I just noticed a couple of minor typos.

chaincfg/params.go Outdated Show resolved Hide resolved
blockchain/thresholdstate.go Outdated Show resolved Hide resolved
blockchain/treasury_policy_test.go Show resolved Hide resolved
Copy link
Member

@JoeGruffins JoeGruffins left a comment

Choose a reason for hiding this comment

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

All looks right to me.

@dnldd
Copy link
Member

dnldd commented Jul 19, 2021

Should be good after resolving @rstaudt2 review issues.

@matheusd
Copy link
Member Author

Fixed @rstaudt2 review issues.

@davecgh davecgh added this to the 1.7.0 milestone Jul 23, 2021
@davecgh davecgh changed the title multi: Prepare for DCP0007 multi: Prepare for DCP0007. Aug 2, 2021
chaincfg/regnetparams.go Show resolved Hide resolved
Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

This looks great overall. I've identified a few minor things inline.

blockchain/thresholdstate.go Outdated Show resolved Hide resolved
blockchain/treasury_policy_test.go Show resolved Hide resolved
blockchain/thresholdstate.go Outdated Show resolved Hide resolved
@matheusd
Copy link
Member Author

matheusd commented Aug 2, 2021

Squashed commits, rebased on top of latest master and fixed latest review items.

Testnet and mainnet deployments were parametrized to run over the following time span:

StartTime:  1631750400, // Sep 16th, 2021
ExpireTime: 1694822400, // Sep 16th, 2023

@davecgh
Copy link
Member

davecgh commented Aug 2, 2021

Confirming the timestamps:

Linux:

$ date --utc --date '@1631750400'
Thu Sep 16 00:00:00 UTC 2021
$ date --utc --date '@1694822400'
Sat Sep 16 00:00:00 UTC 2023

OpenBSD:

$ date -ur 1631750400
Thu Sep 16 00:00:00 UTC 2021
$ date -ur 1694822400
Sat Sep 16 00:00:00 UTC 2023

Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Updates look good. I did spot that the vote was note added to blockchain/fullblocktests/params.go as well. I typically add it there too because those parameters are used in the regression tests and if it hits a path where the deployment is expected to exist, but doesn't, it'll fail unexpectedly.

This adds the deployment and agenda parameters for the DCP0007 vote.
This switches the current blockchain go.mod file to use a replace
directive for the chaincfg code, which is needed due to the new agenda
introduced in a previous commit.
This adds functions to verify the state of voting for the DCP0007
"revert treasury maximum expenditure policy" agenda.
This allows creating tests that require multiple agendas to be activated
simultaneously.
This renames the existing maxTreasuryExpenditure function to
maxTreasuryExpenditureDCP0006 to identify it as the one implementing the
legacy policy.

A future commit will add a new policy and selection of the active one
will be based on the results of an on-chain voting.
This changes the amount used for the large tspend in the rpc treasury
simnet test so that it is compatible to the new expenditure policy and
fails as expected.
@matheusd
Copy link
Member Author

matheusd commented Aug 3, 2021

Added to fullblocktests/params.go. Also added the description to the agenda, which as missing.

@davecgh
Copy link
Member

davecgh commented Aug 3, 2021

Added to fullblocktests/params.go. Also added the description to the agenda, which as missing.

Nice. I noticed the description myself earlier when going over it again, but hadn't commented yet as I was waiting to finish another round of review.

Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

I've given this another round of review and don't see any other issues.

I also modified wallet locally to pull in the latest chaincfg dev module and bump the vote version to ensure things are showing up properly there as well:

$ ./ctl getvotechoices
{
  "version": 10,
  "choices": [
    {
      "agendaid": "reverttreasurypolicy",
      "agendadescription": "Change maximum treasury expenditure policy as defined in DCP0007",
      "choiceid": "abstain",
      "choicedescription": "abstain voting for change"
    }
  ]
}

@davecgh
Copy link
Member

davecgh commented Aug 3, 2021

Also, simnet results of getvoteinfo for reference:

$ ./ctl getvoteinfo 10
{
  "currentheight": 32,
  "startheight": 144,
  "endheight": 463,
  "hash": "03cb3fb5bac92824e20efd6856c3955881e75e584a57951cdbe21b195889ff0f",
  "voteversion": 10,
  "quorum": 160,
  "totalvotes": 0,
  "agendas": [
    {
      "id": "reverttreasurypolicy",
      "description": "Change maximum treasury expenditure policy as defined in DCP0007",
      "mask": 6,
      "starttime": 0,
      "expiretime": 9223372036854775807,
      "status": "defined",
      "quorumprogress": 0,
      "choices": [
        {
          "id": "abstain",
          "description": "abstain voting for change",
          "bits": 0,
          "isabstain": true,
          "isno": false,
          "count": 0,
          "progress": 0
        },
        {
          "id": "no",
          "description": "keep the existing consensus rules",
          "bits": 2,
          "isabstain": false,
          "isno": true,
          "count": 0,
          "progress": 0
        },
        {
          "id": "yes",
          "description": "change to the new consensus rules",
          "bits": 4,
          "isabstain": false,
          "isno": false,
          "count": 0,
          "progress": 0
        }
      ]
    }
  ]
}

@davecgh davecgh merged commit 9b4d8f9 into decred:master Aug 13, 2021
@matheusd matheusd deleted the prep-dcp0007 branch August 13, 2021 12:52
This pull request was closed.
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.

5 participants