search

solana-labs / solana

Web-Scale Blockchain for fast, secure, scalable, decentralized apps and marketplaces.
https://solanalabs.com
Apache License 2.0
13.19k stars 4.3k forks source link

Cross-program balanced instruction handling is too restrictive #9711

Open jackcmay opened 4 years ago

jackcmay commented 4 years ago

Problem

The MessageProcessor's balanced instruction check ensures the Lamport sum of all accounts in the instruction remains the same before and after the instruction. Cross-program invocations only include the accounts required by the invoked instruction and thus leaves out any transfers the invoking program may have made to other accounts before the call to invoke.

For example:

For programs A and B and accounts X, Y, Z

It requires developers to be aware of when they transfer tokens relative to any invocations they are making.

Proposed Solution

Include all accounts in the balance calculations

jackcmay commented 4 years ago

@jstarry

mvines commented 4 years ago

@jackcmay - do you want to keep this in 1.2 or punt to 1.3?

jackcmay commented 4 years ago

@mvines I think given the other priorities punting to 1.3 is more realistic

leoluk commented 3 years ago

Looks like this is breaking Wormhole guardian set upgrades, which - if the subsidizer pool is full - do:

[devnet] [2021-02-01T19:09:41.671877046Z DEBUG solana_runtime::message_processor] Program Bridge1p5gheXUvJ6jGWGeCsgPKgnE3YgdGKRVCMY9o invoke [1]
[devnet] [2021-02-01T19:09:41.672078448Z DEBUG solana_runtime::message_processor] Program log: Instruction: PostVAA
[devnet] [2021-02-01T19:09:41.673453480Z DEBUG solana_runtime::message_processor] Program log: deriving key
[devnet] [2021-02-01T19:09:41.673568861Z DEBUG solana_runtime::message_processor] Program log: deploying contract
[devnet] [2021-02-01T19:09:41.673794033Z DEBUG solana_runtime::message_processor] Program 11111111111111111111111111111111 invoke [2]
[devnet] [2021-02-01T19:09:41.673816843Z TRACE solana_runtime::system_instruction_processor] process_instruction: CreateAccount { lamports: 3814080, space: 420, owner: Bridge1p5gheXUvJ6jGWGeCsgPKgnE3YgdGKRVCMY9o }
[devnet] [2021-02-01T19:09:41.673826873Z TRACE solana_runtime::system_instruction_processor] keyed_accounts: [KeyedAccount { is_signer: true, is_writable: true, key: 6sbzC1eH4FTujJXWj51eQe25cYvr4xfXbJ1vAj7j2k5J, account: RefCell { value: Account { lamports: 99776482800 data.len: 0 owner: 11111111111111111111111111111111 executable: false rent_epoch: 0 } } }, KeyedAccount { is_signer: true, is_writable: true, key: 7h2RXNxM9r6NDvsxRXkn7rtGt2VNe6zcs5b74vbrhjkg, account: RefCell { value: Account { lamports: 0 data.len: 0 owner: 11111111111111111111111111111111 executable: false rent_epoch: 0 } } }]
[devnet] [2021-02-01T19:09:41.673851953Z DEBUG solana_runtime::message_processor] Program 11111111111111111111111111111111 success
[devnet] [2021-02-01T19:09:41.674085315Z DEBUG solana_runtime::message_processor] Program log: deriving key
[devnet] [2021-02-01T19:09:41.674340378Z DEBUG solana_runtime::message_processor] Program log: deploying contract
[devnet] [2021-02-01T19:09:41.674586211Z DEBUG solana_runtime::message_processor] Program Bridge1p5gheXUvJ6jGWGeCsgPKgnE3YgdGKRVCMY9o consumed 100351 of 200000 compute units
[devnet] [2021-02-01T19:09:41.674612091Z DEBUG solana_runtime::message_processor] Program Bridge1p5gheXUvJ6jGWGeCsgPKgnE3YgdGKRVCMY9o BPF VM error: sum of account balances before and after instruction do not match
[devnet] [2021-02-01T19:09:41.674620231Z DEBUG solana_runtime::message_processor] Program Bridge1p5gheXUvJ6jGWGeCsgPKgnE3YgdGKRVCMY9o failed: sum of account balances before and after instruction do not match

Reproduction instructions:

jackcmay commented 3 years ago

@leoluk One workaround is to do all in-program transfers at the end of a program's execution, is that possible here?

billythedummy commented 11 months ago

Does anyone ever intend to fix this? I have not tried it yet but I think this means programs cannot do the following basic procedure:

outdoteth commented 10 months ago

+1 trying to do the exact same use case that @billythedummy speaks about.

transfer + sync native

It's not possible right now, so what is a workaround?

NikaTheEngineer commented 5 months ago

Any updates on this one? I am also trying to:

toptalhook commented 5 months ago

Any updates on this one? I am also trying to:

  • transfer lamports to a wSOL token account by directly decrementing lamports from a program owned account
  • SyncNative the wSOL account

Now, I am also trying to do this. Do you have any resolution?

NikaTheEngineer commented 5 months ago

@toptalhook The only thing you can do is wrap the SOL on a normal(non-program-owned) account and transfer wSOL to the program-owned account.

b1acKr0se commented 4 months ago

Does anyone ever intend to fix this? I have not tried it yet but I think this means programs cannot do the following basic procedure:

  • transfer lamports to a wSOL token account by directly decrementing lamports from a program owned account
  • SyncNative the wSOL account

This is still not possible in 2024. Please, someone take a look.

andreisilviudragnea commented 2 months ago

@jstarry @mvines Do you have any update on this issue?

jstarry commented 2 months ago

To understand how to work around this runtime constraint, it's important to understand that whenever an instruction is processed in the SVM, the latest lamport balance of each instruction account is recorded and tracked. Later when a CPI is made with just a subset of those accounts, it performs balanced lamport check by adding the latest lamports for the subset of accounts passed to the CPI instruction to the sum of the last known lamport balances of the accounts that were passed to the parent instruction but not to the child CPI instruction. This sum must be the same as the sum of all last known account lamport balances when the parent instruction began processing.

So let's say you had an instruction A with accounts X (90 lamports) and Y (10 lamports) which at the beginning of processing had a sum of 100 lamports. Then, instruction A internally moves 10 lamports from account X to account Y and invokes a CPI instruction B with only account Y. When the runtime starts processing instruction B, it doesn't know what changes instruction A made to account X because it only sees the latest state of account Y which was passed as an input to instruction B. So it calculates the latest sum of accounts as 90 + 20 = 110 which is greater than 100. In order to work around this constraint, you can pass account X to the CPI (despite it not being used by instruction B) so that the runtime sees its latest balance of 80 lamports and calculates the correct sum of 100 lamports.

TL;DR: append all accounts with lamport changes to the end of your CPI instruction accounts list.