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: add realloc constraint group #1943

Merged
merged 14 commits into from
Jun 5, 2022
Merged

lang: add realloc constraint group #1943

merged 14 commits into from
Jun 5, 2022

Conversation

callensm
Copy link
Member

@callensm callensm commented Jun 2, 2022

Creates a new constraint group to include the usage of realloc = <new_space>, realloc::payer = <account>, and realloc::zero = <bool> constraints.

Handles the transfer of lamports between the reallocated and allocator accounts based on the delta-bytes (additive vs subtractive reallocation).

closes #1917

@callensm
Copy link
Member Author

callensm commented Jun 2, 2022

open discussion on how to handle the data zeroing option in the AccountInfo::realloc function.

currently set to false, but was wondering if that should also be open to a third constraint in the new group.

@callensm callensm changed the title lang: add realloc constraint group [WIP] lang: add realloc constraint group Jun 2, 2022
@callensm callensm changed the title [WIP] lang: add realloc constraint group lang: add realloc constraint group Jun 2, 2022
@armaniferrante
Copy link
Member

We should add docs on this here https://github.com/project-serum/anchor/blob/master/lang/derive/accounts/src/lib.rs before merging.

@armaniferrante
Copy link
Member

open discussion on how to handle the data zeroing option in the AccountInfo::realloc function.

currently set to false, but was wondering if that should also be open to a third constraint in the new group.

Does zero_init only zeroize the new bytes that are added? Or does it zeroize the entire account?

@callensm
Copy link
Member Author

callensm commented Jun 3, 2022

Does zero_init only zeroize the new bytes that are added? Or does it zeroize the entire account?

Its for zero initializing new memory, however, the docs have this caveat...

Note: Memory used to grow is already zero-initialized upon program entrypoint and re-zeroing it wastes compute units. If within the same call a program reallocs from larger to smaller and back to larger again the new space could contain stale data. Pass true for zero_init in this case, otherwise compute units will be wasted re-zero-initializing.

@armaniferrante
Copy link
Member

armaniferrante commented Jun 3, 2022

@callensm based on that comment it sounds like one would almost never need to pass in that parameter. Nevertheless, I'm thinking we should force the developer to be explicit in their intention and pass in this parameter regardless, so that they at least are forced to read the docs and understand what is happening. It's ugly, but a bit safer in the event there's a reason one actually needs to re-zero-initialize (e.g., they call try_accounts manually in the instruction handler).

@callensm
Copy link
Member Author

callensm commented Jun 3, 2022

@armaniferrante sounds good. next commit will contain resolutions for your suggestions above (renaming allocator to realloc::payer and docs) and adding a third realloc::zero constraint to the group for this rare use-case

__field_info.data_len().checked_sub(#new_space).unwrap()
};

if __delta_space > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

from the solana source code:

    /// Note:  Account data can be increased within a single call by up to
    /// `solana_program::entrypoint::MAX_PERMITTED_DATA_INCREASE` bytes.

perhaps we can give an informative error if delta_space exceeds this? Not sure what that error looks like on the Solana end.

lang/syn/src/parser/accounts/constraints.rs Outdated Show resolved Hide resolved
@armaniferrante armaniferrante merged commit 7fe39c6 into coral-xyz:master Jun 5, 2022
@callensm callensm deleted the ft/realloc branch June 5, 2022 14:22
armaniferrante added a commit that referenced this pull request Jun 17, 2022
armaniferrante added a commit that referenced this pull request Jun 17, 2022
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.

Feature Request: add pda realloc to account macro
4 participants