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: add registry deposits, withdraws #1038

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ananas-block
Copy link
Contributor

No description provided.

}

// Retrieve the event.
// let event = simulation_result
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this can be removed.

Copy link
Contributor

@vadorovsky vadorovsky left a comment

Choose a reason for hiding this comment

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

another round of nits, nothing critical - I'm also fine with skipping the CHECK: stuff and handling it properly later

}
// else if signer_is_delegate.is_some() && input_token_data.delegate_index.is_none() {
Copy link
Contributor

Choose a reason for hiding this comment

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

commented code

input_compressed_accounts: &[CompressedAccount],
input_merkle_context: &[MerkleContext],
input_compressed_accounts: &[CompressedAccountWithMerkleContext],
// input_merkle_context: &[MerkleContext],
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

This file looks empty. Can we remove?

/// CHECK:
#[account(seeds = [CPI_AUTHORITY_PDA_SEED], bump)]
pub cpi_authority: AccountInfo<'info>,
/// CHECK:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// CHECK:
/// CHECK: Seed constraint.

output_pda: OutputCompressedAccountWithPackedContext,
remaining_accounts: Vec<AccountInfo<'info>>,
) -> Result<()> {
// let cpi_context = if let Some(mut cpi_context) = cpi_context {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it needed?

delegate_account.pending_delegated_stake_weight = delegate_account
.pending_delegated_stake_weight
.checked_add(delegate_amount)
.ok_or(RegistryError::ComputeEscrowAmountFailed)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, we should aim for such checked arithmetics everywhere 👍

no_sync,
);

assert!(matches!(result, Err(error) if error == RegistryError::InvalidAuthority.into()));
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, this should also work

Suggested change
assert!(matches!(result, Err(error) if error == RegistryError::InvalidAuthority.into()));
assert!(matches!(result, Err(RegistryError::InvalidAuthority)));

(no need to change, just saying)

refactor: allow delegate mix its own undelegated accounts with delegated accounts

deposit functional tests work

claiming works, delegation doesnt work

undelegate works with 1 delegate
@ananas-block ananas-block force-pushed the jorrit/feat-add-registry-deposits branch from f67801d to 56d1927 Compare August 2, 2024 01:19
@ananas-block ananas-block marked this pull request as draft August 3, 2024 01:41
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