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

Start Conway Imp tests with an initial committee and constitution #4285

Merged
merged 7 commits into from
Apr 26, 2024

Conversation

teodanciu
Copy link
Contributor

@teodanciu teodanciu commented Apr 21, 2024

Description

This PR is setting an initial committee and constitution for Conway (emulating the ones that will be defined in genesis).
For this, it was necessary to slightly modify the initialization of the NewEpochState in the Imp framework, to allow initialization actions before returning the state.

Tests were adjusted to only elect a committee if this action is actually meaningful to the test (relying on the initial committee otherwise).
Going through many tests was an opportunity to make some small improvements:

  • an attempt to be more disciplined with setting pparams. I have consolidated a few of them
  • updating some functions to reduce the noise of converting between KeyHash and Credential
  • removing the duplicated inclusion of a test

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated
  • When applicable, versions are updated in .cabal and CHANGELOG.md files according to the
    versioning process.
  • The version bounds in .cabal files for all affected packages are updated. If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)
  • All visible changes are prepended to the latest section of a CHANGELOG.md for the affected packages. New section is never added with the code changes. (See RELEASING.md)
  • Code is formatted with fourmolu (use scripts/fourmolize.sh)
  • Cabal files are formatted (use scripts/cabal-format.sh)
  • hie.yaml has been updated (use scripts/gen-hie.sh)
  • Self-reviewed the diff

@teodanciu teodanciu changed the title Start Conway Imp tests with an initial committe Start Conway Imp tests with an initial committee Apr 21, 2024
@teodanciu teodanciu marked this pull request as draft April 21, 2024 11:23
@teodanciu teodanciu force-pushed the td/imptest-initial-committee branch 10 times, most recently from 6a211ca to bfeaed7 Compare April 23, 2024 02:24
@teodanciu teodanciu marked this pull request as ready for review April 23, 2024 02:31
@teodanciu teodanciu force-pushed the td/imptest-initial-committee branch 2 times, most recently from c5a1b7b to 3fbfdd9 Compare April 23, 2024 15:38
@teodanciu teodanciu requested a review from lehins April 23, 2024 16:53
Copy link
Contributor

@lehins lehins left a comment

Choose a reason for hiding this comment

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

Looks good, but committee initialization is a bit hacky and it seems that thresholds got relaxed for many tests

@teodanciu teodanciu force-pushed the td/imptest-initial-committee branch from 3fbfdd9 to 6a93d51 Compare April 25, 2024 21:13
@teodanciu teodanciu changed the title Start Conway Imp tests with an initial committee Start Conway Imp tests with an initial committee and constitution Apr 25, 2024
@teodanciu teodanciu force-pushed the td/imptest-initial-committee branch from 6a93d51 to 6e296c2 Compare April 25, 2024 21:37
@teodanciu
Copy link
Contributor Author

  • Removed global bindings for committee, and generated them with freshKeyPair instead
  • Set voting thresholds when initializing and adjusted the tests accordingly
  • Added initial constitution

If you want to have another look @lehins when you can, thanks!

@teodanciu teodanciu requested a review from lehins April 25, 2024 21:39
@teodanciu teodanciu force-pushed the td/imptest-initial-committee branch from 6e296c2 to 289e09c Compare April 25, 2024 21:58
Copy link
Contributor

@lehins lehins left a comment

Choose a reason for hiding this comment

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

Looks great. Thank you!

* set sensible values when initializing NewEpochState
* remove duplicated modifications of the same parameters
@teodanciu teodanciu force-pushed the td/imptest-initial-committee branch from e174283 to 1f80b22 Compare April 26, 2024 08:52
@teodanciu teodanciu merged commit 2e3f1be into master Apr 26, 2024
122 of 123 checks passed
@teodanciu teodanciu deleted the td/imptest-initial-committee branch April 26, 2024 10:24
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.

None yet

2 participants