SystemInstruction::Deallocate

[jon-chuang]

Problem

The current idiom applied in the Solanaverse for closing accounts is to defund the account, whereupon it will be loaded as an empty account for subsequent transactions.

This is because if address has lamports == 0, Bank.accounts returns AccountSharedData::default() for all loading paths.

However, within the same transaction, this account address could either be hijacked or kept in permanent limbo through various means by other instructions.

Proposed Solution

Expose SystemInstruction::Deallocate, which can instantly set the account to be equivalent to AccountSharedData::default(), i.e. with 0 lamports and len 0 data slice. This forces further use of that address to obey the rules of authorisation for system accounts.

Discussion

There are several directions possible.

1. The calling program needs to set the system program to be the account owner, and thus must first zero out the account data, before calling SystemInstruction::Deallocate. Currently, zeroing out the account data is probably cheapest to do with a sol_memset_, which does the following thing, and is hopefully turned into an efficient implementation by the rust compiler:

let s = question_mark!(
    translate_slice_mut::<u8>(memory_mapping, s_addr, n, self.loader_id, true),
    result
);
for val in s.iter_mut().take(n as usize) {
    *val = c as u8;
}

Although we know for sure that std::ptr::write_bytes does a memset under the hood. According to this stackoverflow thread, the compiler can do memset for large arrays for the above code snippet already.

Testing this, here is the rust.godbolt output with the -O flag:

pub fn thing(buffer: &mut [u8]) {
    for i in &mut buffer[..] { *i = 240 }
}

example::thing:
        test    rsi, rsi
        je      .LBB1_2
        push    rax
        mov     rdx, rsi
        mov     esi, 240
        call    qword ptr [rip + memset@GOTPCREL]
        add     rsp, 8

So I think we're good.

This costs at the current default compute budget is 1 compute unit per 250 bytes, so a grand total of 40,000 CUs per 10MB, definitely very doable.

2. The calling program can directly call the CPI SystemInstruction::Deallocate. This merely checks that the calling program is the owner of the account. However, since the system program needs to become the new owner, this requires changes to the verify step in the runtime/MessageProcessor. To reduce attack surface, this is not recommended.

In both cases, the account can either have zero lamports or the instruction can specify an account to which to drain remaining lamports. To reduce the complexity in the runtime, I recommend that the lamports must be set to 0 already.

Alternately, lamports does not have to be set to 0 post-deallocation, there is nothing strictly wrong with that. It'll just be a funded system account with len 0 data.

Related

coral-xyz/anchor#604
#19205

@jstarry @joncinque @armaniferrante

[jon-chuang]

I was also thinking that a convenience function that does the lamports transfer, zeroing of data via sol_memset, change of ownership (requires unsafe), and finally making the SystemInstruction::Deallocate CPI call could be exposed to the user.

Similar to how invoke_signed is also such an interface.

I was thinking this lives in sdk::program and has the following APIs:

fn close_account(
    account: &AccountInfo, 
    system_program: &AccountInfo,
    account_signer_seed: Option<&[[u8]]>,
) -> ProgramResult {
    // Reassign address to the system program

    // This is safe since
    // 1. bpf VM is single-threaded
    // 2. KeyedAccount's pubkey is serialized to BPF's INPUT buffer per VM
    unsafe {
        let const_ptr = account.owner as *const Pubkey;
        let mut_ptr = const_ptr as *mut Pubkey;
        *mut_ptr = system_program::id();
    }

    // zero the data as this is required to transfer ownership
    let mut data = info.try_borrow_mut_data()?;
    let data_len = data.len();
    sol_memset(data, 0u8, data_len);

    let ix = solana_program::system_instruction::deallocate(account.key);
    if let Some(signer_seeds) = account_signer_seeds {
        invoke_signed(&ix, &[account, system_program], &[signer_seed])
    } else {
        invoke(&ix, &[account, system_program])
    }
}

fn close_and_defund_account(
    account: &AccountInfo, 
    lamports_destination: &AccountInfo,
    system_program: &AccountInfo,
    account_signer_seed: Option<&[[u8]]>,
) -> ProgramResult {
    let ix = solana_program::system_instruction::transfer(account.key, destination.key, account.lamports());
    if let Some(signer_seeds) = account_signer_seeds {
        invoke_signed(&ix, &[account, lamports_destination, system_program], &[signer_seed])?;
    } else {
        invoke(&ix, &[account, lamports_destination, system_program])?;
    }
    close_account(account, system_program, account_signer_seed)
}

[jon-chuang]

Actually @jstarry , would you be able to shed light on why SystemInstruction::Allocate cannot simply play this role by allowing accounts with data already allocated to be reallocated?

Is that because the nonce account is itself a system program account? Perhaps this wasn't the wisest choice and the namespace of the system program should have been reserved for account allocations/creation.

Perhaps what we actually need is for Allocate to check if the system program owned account is a nonce account and allow for reallocations otherwise? In which case we wouldn't need Deallocate, we'd do the above and call Allocate(0)

[jon-chuang]

I've actually identified a final solution that could be most pleasing.

That is account resizing via CPI by owning programs. So the calling program first resizes the account to 0. Then, the calling program can transfer ownership to the system account.

However, I know this has been talked about quite a bit and there are several blockers on this, although I don't know what the blockers are.

These blockers would probably also apply to system account re-allocation, so maybe you'd be able to advise here.

[jstarry]

the calling program first resizes the account to 0. Then, the calling program can transfer ownership to the system account.

I'm in favour of this approach, I think it's reasonable for programs to have control over the data size of the accounts they own. I'm not aware of any blockers to adding support for that functionality to the runtime.

[jstarry]

@jackcmay will have a better idea of any potential blockers

[jackcmay]

Deallocation to size zero I think should be doable today with a little work, will need to take a closer look

[jackcmay]

Deallocation to size zero I think should be doable today with a little work, will need to take a closer look

WIP PR that allows programs to do this: #19475