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: protocol config #986

Merged
merged 4 commits into from
Jul 31, 2024
Merged

feat: protocol config #986

merged 4 commits into from
Jul 31, 2024

Conversation

ananas-block
Copy link
Contributor

@ananas-block ananas-block commented Jul 15, 2024

Conceptual Changes:

  • foresters need to be registered with forester_accounts
  • foresters need to register to epochs to forest

Changes:

  • refactor registry program folder structure
    • moved everything from the decentralization and contention prevention test into the program
    • selection is not used yet
    • claim functionality is not implemented yet

Note:

  • security and failing tests are not implemented yet
  • rewards are not implemented yet
  • implementation is insecure, security checks are missing
  • leaving msg! prints in registry program for now for debugging

@ananas-block ananas-block changed the title feat: protocol config and epochs feat: protocol config Jul 15, 2024
@ananas-block ananas-block force-pushed the jorrit/feat-protocol-config branch 3 times, most recently from 8fb5872 to 8d8dc00 Compare July 21, 2024 21:19
create_rollover_address_merkle_tree_instruction, create_rollover_state_merkle_tree_instruction,
CreateRolloverMerkleTreeInstructionInputs,
};
// use light_registry::sdk::{
Copy link
Member

Choose a reason for hiding this comment

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

Can we just remove these imports?

/// Anyone can create new trees just the fees cannot be set arbitrarily.
#[account(mut)]
pub authority: Signer<'info>,
/// CHECK:
Copy link
Member

Choose a reason for hiding this comment

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

Could we at least add something like

Suggested change
/// CHECK:
/// CHECK: Is checked in the account-compression program during CPI.

in all fields where Anchor requires CHECK docstrings? I know it's obvious for us, but some context would be helpful (for auditors as well).


#[derive(Accounts)]
pub struct NullifyLeaves<'info> {
/// CHECK:
Copy link
Member

Choose a reason for hiding this comment

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

same here

Comment on lines 21 to 22
#[account(mut)]
#[account(seeds = [CPI_AUTHORITY_PDA_SEED], bump)]
Copy link
Member

Choose a reason for hiding this comment

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

First of all, does CPI authority need to be mutable here?

I don't think it should be necessary, but if I'm missing something and it needs to be mutable, could we merge these into one #[account] attribute?

Suggested change
#[account(mut)]
#[account(seeds = [CPI_AUTHORITY_PDA_SEED], bump)]
#[account(mut, seeds = [CPI_AUTHORITY_PDA_SEED], bump)]

pub account_compression_program: Program<'info, AccountCompression>,
/// CHECK: when emitting event.
pub log_wrapper: UncheckedAccount<'info>,
/// CHECK: in account compression program
Copy link
Member

Choose a reason for hiding this comment

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

or, well, even such comment is sufficient, I would be fine with copying it to the empty CHECK: fields


#[derive(Accounts)]
pub struct RolloverMerkleTreeAndQueue<'info> {
/// CHECK:
Copy link
Member

Choose a reason for hiding this comment

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

same here

refactor: registry program add forester selection and contention

fix rollover test, regenerate idl
@vadorovsky vadorovsky merged commit 9902c37 into main Jul 31, 2024
14 checks passed
@vadorovsky vadorovsky deleted the jorrit/feat-protocol-config branch July 31, 2024 07:17
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.

2 participants