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

lang: adjust realloc implementation to safeguard max increase and idempotency #1986

Merged
merged 14 commits into from
Jul 5, 2022
Merged

Conversation

callensm
Copy link
Member

@callensm callensm commented Jun 18, 2022

  • includes the original implementation of the realloc constraint group from lang: add realloc constraint group #1943
  • adds a check on the __delta_bytes to ensure it is <= MAX_PERMITTED_DATA_INCREASE
  • add test to ensure duplicate accounts does not imply duplicate reallocation
  • adds new AccountReallocExceedsLimit error code to rust and typescript for the data increase limit check

@callensm callensm changed the title lang: adjust realloc implementation to safeguard max increase and idempotency [WIP] lang: adjust realloc implementation to safeguard max increase and idempotency Jun 18, 2022
@callensm callensm changed the title [WIP] lang: adjust realloc implementation to safeguard max increase and idempotency lang: adjust realloc implementation to safeguard max increase and idempotency Jun 18, 2022
@callensm
Copy link
Member Author

added reallocs: &mut std::collections::BTreeSet<String> as a new argument to Accounts::try_accounts in order to track public keys that are assigned to reallocation constraint groups in order to throw a duplicate account realloc error if the public key of an account is associated with more than one reallocation per instruction call.

Copy link

@ethanwu10 ethanwu10 left a comment

Choose a reason for hiding this comment

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

It seems that coverage of #[state] is lacking for zerocopy. Also, the second typo in the ix.has_receiver case for state interface impls is dead (and thus also untested), since #[interface] does not allow receivers in any of the trait's methods.

Also, I didn't see anything in rustdoc indicating that #[state] is deprecated...

lang/syn/src/codegen/program/handlers.rs Outdated Show resolved Hide resolved
lang/syn/src/codegen/program/handlers.rs Outdated Show resolved Hide resolved
@callensm
Copy link
Member Author

tests/swap is passing when i run it locally with the debug build of the cli, not sure why its failing in the pipeline.

lang/src/lib.rs Outdated
@@ -82,6 +82,7 @@ pub trait Accounts<'info>: ToAccountMetas + ToAccountInfos<'info> + Sized {
accounts: &mut &[AccountInfo<'info>],
ix_data: &[u8],
bumps: &mut BTreeMap<String, u8>,
reallocs: &mut BTreeSet<String>,

Choose a reason for hiding this comment

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

Since AFAICT nothing in docs signals that the Accounts trait is internal, this is a breaking change (with no real way around it if we want to implement the duplicate checking). It'd probably be a better idea to pass a new &mut TryAccountsContext struct or similar so that we can add more bits of state or data without needing to further modify this signature. Consider:

#[derive(Default)]
pub struct TryAccountsContext {
    // private field to prevent the struct from containing only public fields; prevents users from
    // constructing using a literal or pattern-matching/destructuring against this type
    _marker: (),
    pub bumps: BTreeMap<String, u8>,
    pub reallocs: BTreeSet<String>,
}

lang/src/lib.rs Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@vercel
Copy link

vercel bot commented Jul 4, 2022

@armaniferrante is attempting to deploy a commit to the 200ms Team on Vercel.

A member of the Team first needs to authorize it.

@edmbn
Copy link

edmbn commented Jul 4, 2022

added reallocs: &mut std::collections::BTreeSet<String> as a new argument to Accounts::try_accounts in order to track public keys that are assigned to reallocation constraint groups in order to throw a duplicate account realloc error if the public key of an account is associated with more than one reallocation per instruction call.

Will this mean we won't be able to reallocate an account 2 times in the same instruction? We might want to reallocate an account with 0 space and then with try_accounts reallocate to original space to have the account "reinitialized".

@armaniferrante armaniferrante merged commit c47fb28 into coral-xyz:master Jul 5, 2022
@callensm callensm deleted the fix/realloc branch July 5, 2022 19:54
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