CLI: Auto-extend program accounts by default

[Nagaprasadvr]

issue #564

Problem

When you deploy a program, it initially takes up 2x the program size. At a later date if you try to deploy larger than the currently allocated size, you get the following error:

Error: Deploying program failed: Error processing Instruction 0: account data too small for instruction
The correct way to solve this is to use solana program extend <PROGRAM_ID> <MORE_BYTES> to extend the program, then deploy it again.

Developers attempting to calculate this amount over and over again have become frustrated and some have just spent the money to allocate way more than they should. Some even as much as 10mb. This is bad both for the cluster and developer experience.

Summary of Changes

1. Add Checks in do_process_program_upgrade fn to calculate additional bytes required to extend the program and throw an error if auto-extend-program flag is not passed with message to use solana program extend
2. Add --auto-extend-program flag to solana program deploy command and use this to auto extend solana program data account in case of overflow

[buffalojoec]

Nice! Not a bad idea, and thanks for rolling tests and nice output logs.

I'm wondering if we should just make this the default behavior, rather than requiring an option to trigger it. It seems to me like 10/10 times the user who encounters that error message instructing them to use --auto-extend-program is going to immediately retry with the option.

Instead, your program data account will always be extended automatically on deployment. Maybe we can also offer an option for --no-extend to do opposite, for those rare cases where someone might want that.

What do you think?

[Nagaprasadvr]

Nice! Not a bad idea, and thanks for rolling tests and nice output logs.

I'm wondering if we should just make this the default behavior, rather than requiring an option to trigger it. It seems to me like 10/10 times the user who encounters that error message instructing them to use --auto-extend-program is going to immediately retry with the option.

Instead, your program data account will always be extended automatically on deployment. Maybe we can also offer an option for --no-extend to do opposite, for those rare cases where someone might want that.

What do you think?

Yeah sounds good to me , I can change logic to auto extend and add no-extend flag as well

[Nagaprasadvr]

@buffalojoec have changed the logic

[buffalojoec]

Nice! Thanks for reworking that. I left a couple of comments!

I haven't reviewed the test you've written yet, but do you think it makes sense to also include a test where we introduce a program upgrade that's clearly larger than the existing program, and verify it succeeds?

[Nagaprasadvr]

Nice! Thanks for reworking that. I left a couple of comments!

I haven't reviewed the test you've written yet, but do you think it makes sense to also include a test where we introduce a program upgrade that's clearly larger than the existing program, and verify it succeeds?

okay i have removed that test now , as we auto extend the program data account

[buffalojoec]

I know you're trying to give the user a little log message about the program size being too small, but since we've flopped this option around to be explicitly "do not attempt to extend", I think it's fine to allow the transaction to fail as it currently does, if --no-extend is provided.

With that being said, I think you can simplify this approach by sliding the block of code that does the program data account lookup down into the initial_instructions setup step.

Basically, you could condense everything you've changed in do_process_program_upgrade into something like this and place it on line 2521.

if !no_extend {
    // Attempt to look up the existing program's size, and automatically
    // add an extend instruction if the program data account is too small.
    let program_data_address = get_program_data_address(program_id);
    if let Some(program_data_account) = rpc_client
        .get_account_with_commitment(&program_data_address, config.commitment)?
        .value
    {
        let program_len = UpgradeableLoaderState::size_of_programdata(program_len);
        let account_data_len = program_data_account.data.len();
        if program_len > account_data_len {
            let additional_bytes = program_len.sub(account_data_len);
            let additional_bytes: u32 = additional_bytes.try_into().map_err(|_| {
                format!(
                    "Cannot auto-extend Program Data Account space due to size limit \
                        please extend it manually with command `solana program extend {} \
                        <BYTES>`. Additional bytes required: {}",
                    program_id, additional_bytes
                )
            })?;
            initial_instructions.push(bpf_loader_upgradeable::extend_program(
                program_id,
                Some(&fee_payer_signer.pubkey()),
                additional_bytes,
            ));
        }
    }
}

What do you think?

P.S. clippy is yelling at you about something!

[Nagaprasadvr]

I know you're trying to give the user a little log message about the program size being too small, but since we've flopped this option around to be explicitly "do not attempt to extend", I think it's fine to allow the transaction to fail as it currently does, if --no-extend is provided.

With that being said, I think you can simplify this approach by sliding the block of code that does the program data account lookup down into the initial_instructions setup step.

Basically, you could condense everything you've changed in do_process_program_upgrade into something like this and place it on line 2521.

if !no_extend {
    // Attempt to look up the existing program's size, and automatically
    // add an extend instruction if the program data account is too small.
    let program_data_address = get_program_data_address(program_id);
    if let Some(program_data_account) = rpc_client
        .get_account_with_commitment(&program_data_address, config.commitment)?
        .value
    {
        let program_len = UpgradeableLoaderState::size_of_programdata(program_len);
        let account_data_len = program_data_account.data.len();
        if program_len > account_data_len {
            let additional_bytes = program_len.sub(account_data_len);
            let additional_bytes: u32 = additional_bytes.try_into().map_err(|_| {
                format!(
                    "Cannot auto-extend Program Data Account space due to size limit \
                        please extend it manually with command `solana program extend {} \
                        <BYTES>`. Additional bytes required: {}",
                    program_id, additional_bytes
                )
            })?;
            initial_instructions.push(bpf_loader_upgradeable::extend_program(
                program_id,
                Some(&fee_payer_signer.pubkey()),
                additional_bytes,
            ));
        }
    }
}

What do you think?

P.S. clippy is yelling at you about something!

sounds good to me ! ,i have resolved this

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 29.16667% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 81.9%. Comparing base (f133697) to head (e225def).
Report is 6 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #791   +/-   ##
=======================================
  Coverage    81.9%    81.9%           
=======================================
  Files         855      855           
  Lines      232134   232157   +23     
=======================================
+ Hits       190134   190160   +26     
+ Misses      42000    41997    -3     

[buffalojoec]

Hey @Nagaprasadvr try running:

cargo clippy --workspace --tests --bins --examples --features dummy-for-ci-check -- --deny=warnings --deny=clippy::default_trait_access --deny=clippy::arithmetic_side_effects --deny=clippy::manual_let_else --deny=clippy::used_underscore_binding

[buffalojoec]

Awesome work! I went ahead and added the test for you :)

Thanks for contributing!

[Nagaprasadvr]

Awesome work! I went ahead and added the test for you :)

Thanks for contributing!

your welcome , looking forward to contribute more