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

Add generic builder #1810

Merged
merged 6 commits into from
Aug 1, 2024
Merged

Add generic builder #1810

merged 6 commits into from
Aug 1, 2024

Conversation

shenkeyao
Copy link
Member

@shenkeyao shenkeyao commented Aug 1, 2024

Closes EspressoSystems/marketplace-builder-core#36.
Note: This PR is to be merged to ag/builder-bin and will be merged to main as part of #1806.

This PR:

  • Adds hooks for generic builder.
    • Generic builder is named as "fallback builder".
    • Existing builder is renamed as "reserve builder".
  • Adds a flag to indicate the builder type upon initiation.
  • Updates yaml files.

This PR does not:

  • Change existing builder logic.

Key places to review:

  • builder.rs.
  • hooks.rs.

Copy link
Contributor

@QuentinI QuentinI left a comment

Choose a reason for hiding this comment

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

LGTM!

@QuentinI
Copy link
Contributor

QuentinI commented Aug 1, 2024

@shenkeyao @elliedavidson wdyt of making reserve and fallback non-exclusive options? I.e. allowing the builder binary to be run with any combination of those? I imagine having a --fallback flag that enables fallback behaviour, i.e. when filtering transactions also requesting list of registered namespaces and including all non-registered transactions. And if one wanted to run purely fallback builder, they'd just set namespace list to empty.

@elliedavidson
Copy link
Member

@shenkeyao @elliedavidson wdyt of making reserve and fallback non-exclusive options? I.e. allowing the builder binary to be run with any combination of those? I imagine having a --fallback flag that enables fallback behaviour, i.e. when filtering transactions also requesting list of registered namespaces and including all non-registered transactions. And if one wanted to run purely fallback builder, they'd just set namespace list to empty.

Yes, that makes sense to me.

@shenkeyao
Copy link
Member Author

@shenkeyao @elliedavidson wdyt of making reserve and fallback non-exclusive options? I.e. allowing the builder binary to be run with any combination of those? I imagine having a --fallback flag that enables fallback behaviour, i.e. when filtering transactions also requesting list of registered namespaces and including all non-registered transactions. And if one wanted to run purely fallback builder, they'd just set namespace list to empty.

Sounds good! I’ll make the change.

marketplace-builder/src/builder.rs Outdated Show resolved Hide resolved
This reverts commit 6eafa5b.
@shenkeyao shenkeyao merged commit 7283478 into ag/builder-bin Aug 1, 2024
13 checks passed
@shenkeyao shenkeyao deleted the keyao/generic-builder branch August 1, 2024 21:15
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.

3 participants