Navigation Menu

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: constitution publishing #4150

Merged
merged 13 commits into from Jun 7, 2022

Conversation

brianp
Copy link
Contributor

@brianp brianp commented May 31, 2022

Description

Update the committee proposals into constitution publishing.

Motivation and Context

Meet the new requirements for proposing a committee. Update old code to match new specifications. Refactor the existing functionality instead of introducing net new functionality.

It may have been a better choice to specify a json file as a constitution but I wanted to keep similar functionality to how it was being done. By the end of the PR this was obviously a less ideal choice. We accept 4 arguments to create the constitution at the moment. Where as there are more arguments that can be utilized.

How Has This Been Tested?

  • With CI passing.
  • With a new integration test.
  • Manually using the following command:
    cargo run --bin tari_console_wallet -- -b . --command "publish-constitution-definition --json-file ../../integration_tests/fixtures/constitution_definition.json"

@mrnaveira
Copy link
Contributor

I think it's worth changing all the old "committee definition" terminology for the newer "contract constitution" everywhere, to avoid name inconsistency.

Otherwise, LGTM

@brianp brianp changed the title fix: wip constitution publishing feat: constitution publishing Jun 2, 2022
@brianp brianp marked this pull request as ready for review June 2, 2022 10:09
@brianp brianp marked this pull request as draft June 2, 2022 12:44
@brianp brianp marked this pull request as ready for review June 2, 2022 18:44
Copy link
Collaborator

@SWvheerden SWvheerden 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.

Copy link
Collaborator

@stringhandler stringhandler left a comment

Choose a reason for hiding this comment

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

I'm happy with it, but waiting for #4162 before merging

@aviator-app aviator-app bot merged commit ba83b8f into tari-project:development Jun 7, 2022
@brianp brianp deleted the constitution-publish branch February 13, 2023 20:51
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

5 participants