Use Create Account Checked & Create Account with Seed Checked

[zubayr1]

Instead of passing lamports as part of arguments, passing Sysvar rent account.

So forcing user to pass sysvar_rent_account instead of doing:
lamports: Rent::get()?.minimum_balance(<STATE>::LEN)

where Rent::get() makes a sysvar call directly from runtime, which makes it consume more CUs.

Besides, by internally computing lamports, the code doesn't allow devs to use excess or less lamports

[sonicfromnewyoke]

nice functionality, gives more safety
what do you think about one/two extra calls instead of the full copy/pasted file like so?

#[inline(always)]
pub fn invoke_signed_checked(&self, rent_account_info: &AccountInfo, signers: &[Signer]) -> ProgramResult {
    // getting lamports from rent
    let rent = Rent::from_account_info(rent_account_info?;
    let lamports = rent.minimum_balance(self.space as usize);

    // checking if the funding account has enough lamports
    if self.from.lamports() < lamports {
        return Err(ProgramError::InsufficientFunds);
    }

     self.invoke_signed(signers)
}

[zubayr1]

nice functionality, gives more safety what do you think about one/two extra calls instead of the full copy/pasted file like so?

#[inline(always)]
pub fn invoke_signed_checked(&self, rent_account_info: &AccountInfo, signers: &[Signer]) -> ProgramResult {
    // getting lamports from rent
    let rent = Rent::from_account_info(rent_account_info?;
    let lamports = rent.minimum_balance(self.space as usize);

    // checking if the funding account has enough lamports
    if self.from.lamports() < lamports {
        return Err(ProgramError::InsufficientFunds);
    }

     self.invoke_signed(signers)
}

Since we are making the invoke_signed_check a public function, there is no point making the rent_account_info part of the arguments (since its already part of the struct). Maybe e4e9837 is something you are looking for?

[sonicfromnewyoke]

nice functionality, gives more safety what do you think about one/two extra calls instead of the full copy/pasted file like so?

#[inline(always)]
pub fn invoke_signed_checked(&self, rent_account_info: &AccountInfo, signers: &[Signer]) -> ProgramResult {
    // getting lamports from rent
    let rent = Rent::from_account_info(rent_account_info?;
    let lamports = rent.minimum_balance(self.space as usize);

    // checking if the funding account has enough lamports
    if self.from.lamports() < lamports {
        return Err(ProgramError::InsufficientFunds);
    }

     self.invoke_signed(signers)
}

Since we are making the invoke_signed_check a public function, there is no point making the rent_account_info part of the arguments (since its already part of the struct). Maybe e4e9837 is something you are looking for?

my point was in not copy/pasting the whole create_account.rs file just for adding one field in the struct to perform the check.

this is more DRY to add the wrapper for fn invoke_signed(), isn't it?

[febo]

Have you consider adding a "constructor" method instead? This would minimize the code duplication.

pub fn with_rent_account(
        from: &AccountInfo,
        to: &AccountInfo,
        rent: &AccountInfo,
        space: u64,
        owner: &Pubkey,
    ) -> Result<Self, ProgramError>;

[zubayr1]

Have you consider adding a "constructor" method instead? This would minimize the code duplication.

pub fn with_rent_account(
        from: &AccountInfo,
        to: &AccountInfo,
        rent: &AccountInfo,
        space: u64,
        owner: &Pubkey,
    ) -> Result<Self, ProgramError>;

Please have a look at c709b11

[febo]

I am not too sure if we need this checks – the CPI will do this validation and raised the appropriate error.

[febo]

Same here, might be better to remove this checks.

[febo]

I prefer this version than the previous. One point is whether we need the validation or not, since the CPI will do that. Can you measure the difference is CUs? Ideally we would like the "original" version, rent check (no validation), rent check + validation.

[zubayr1]

I prefer this version than the previous. One point is whether we need the validation or not, since the CPI will do that. Can you measure the difference is CUs? Ideally we would like the "original" version, rent check (no validation), rent check + validation.

Please have a look at: 68cf5be

I checked with the pinocchio starter code https://github.com/Nagaprasadvr/solana-pinocchio-starter (initialize_state.rs but with only codes relevant to the test).
Keeping everything else same except only the necessary codes for the different scenarios:

CreateAccount::with_rent_check (forcing rent to be calculated efficiently internally in CreateAccount): 3073 of 1400000 compute units
CreateAccount (with lamports and with rent = Rent::from_account_info(sysvar_rent_acc)?): 3075 of 1400000 compute units
CreateAccount (with lamports but with rent = Rent::get()?) : 3177 of 1400000 compute units

[febo]

Final nit.

[febo]

Looks good, thanks!