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

Refactor challenger to be in main folder #689

Merged
merged 5 commits into from
Jun 1, 2022
Merged

Conversation

daveroga
Copy link
Contributor

Change Challenger to an independent folder in the root of the solution. Expecting to release scripts to help external parties deploy their own version of challengers. Remove it from cli folder.

There is no change in functionality.

@ChaitanyaKonda
Copy link
Contributor

Similar to user inside test file or user as wallet, challenger/proposer apps are not core nightfall and are part of and only used for testing. So challenger/proposer app should go under test folder?

Usually, developers of nightfall would build their own version of challenger/proposer app using nightfall SDK and use it with the core nightfall (nightfall-client/wallet, nightfall-optimist).

@druiz0992
Copy link
Contributor

Answering @ChaitanyaKonda, current proposer/challenger location makes it extremely difficult to guide an external party on how to build one proposer/challenger. I understand these are not core roles, but certainly cannot be left within a test subfolder. I am fine creating a new folder named apps or roles where we create these roles (proposer, challenger, liquitidy provider, synchronizer,....). I am open to suggestions

@signorecello
Copy link
Contributor

I 200% agree with @druiz0992 , even if we don't consider proposer/challenger as core parties, a test folder isn't definitely the right place for them to be. We'll definitely find a better solution but for now I 200% agree we should have them in a place where external parties can easily identify and use

Copy link
Contributor

@signorecello signorecello left a comment

Choose a reason for hiding this comment

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

LGTM

@daveroga
Copy link
Contributor Author

daveroga commented Jun 1, 2022

Without a proposer you can't make blocks. So it's not the core but very important for the users to understand how to build a proposer and test folder doesn't seem the proper folder.
Challengers and Liquidity providers were not in test but cli folder. This cli folder problably is the one that should be renamed as app or roles as sugested by @druiz0992 for this purpose and structured in the proper way.

@druiz0992 druiz0992 merged commit c6baa42 into master Jun 1, 2022
@druiz0992 druiz0992 deleted the daveroga/challenger branch June 1, 2022 11: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.

5 participants