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

Protocol parameter update with guard rail script #5834

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

Jimbo4350
Copy link
Contributor

Description

Add your description here, if it fixes a particular issue please provide a
link
to the issue.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. These may include:
    • golden tests
    • property tests
    • roundtrip tests
    • integration tests
      See Runnings tests for more details
  • Any changes are noted in the CHANGELOG.md for affected package
  • The version bounds in .cabal files are updated
  • CI passes. See note on CI. The following CI checks are required:
    • Code is linted with hlint. See .github/workflows/check-hlint.yml to get the hlint version
    • Code is formatted with stylish-haskell. See .github/workflows/stylish-haskell.yml to get the stylish-haskell version
    • Code builds on Linux, MacOS and Windows for ghc-8.10.7 and ghc-9.2.7
  • Self-reviewed the diff

Note on CI

If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG node developers to do this
for you.

@Jimbo4350 Jimbo4350 requested review from a team as code owners May 9, 2024 19:37
@Jimbo4350 Jimbo4350 force-pushed the jordan/protocol-param-update-with-guardrail branch 5 times, most recently from d1fdfc3 to efe6f73 Compare May 16, 2024 14:20
Comment on lines +34 to +35
-- program-options
-- ghc-options: -Werror
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
-- program-options
-- ghc-options: -Werror
program-options
ghc-options: -Werror

Comment on lines +81 to +90
H.writeFile proposalAnchorFile "dummy anchor data"
proposalAnchorDataBS <- evalIO $ BS.readFile proposalAnchorFile
Copy link
Contributor

Choose a reason for hiding this comment

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

why write then read?

, Map.singleton pv2 defaultV2CostModel
, Map.singleton pv3 defaultV3CostModel
]
defaultV1CostModel = Api.CostModel
Copy link
Contributor

Choose a reason for hiding this comment

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

are those V1 and V2 values the same as in default genesis in cardano-api? Maybe it would be better to reference values from the default genesis there instead of redefining them here?

Same with V3 - maybe it would be better to just update single elements in the list instead?

numVotes = length allVotes
annotateShow numVotes

guardRailScript <- H.note $ work </> "guard-rail-script.plutusV3"
Copy link
Contributor

Choose a reason for hiding this comment

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

Just out of curiosity, what does this script do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing it defaults to True. I want to bring plutus-example into cardano-node so we can see the script in in Haskell.

@Jimbo4350 Jimbo4350 force-pushed the jordan/protocol-param-update-with-guardrail branch from efe6f73 to 0586d27 Compare May 20, 2024 16:22
@Jimbo4350 Jimbo4350 force-pushed the jordan/protocol-param-update-with-guardrail branch 2 times, most recently from cefd5a6 to cc79ef6 Compare May 28, 2024 18:48
@Jimbo4350 Jimbo4350 force-pushed the jordan/protocol-param-update-with-guardrail branch from cc79ef6 to 4655aa1 Compare June 14, 2024 10:58
Copy link

github-actions bot commented Aug 4, 2024

This PR is stale because it has been open 45 days with no activity.

@github-actions github-actions bot added the Stale label Aug 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants