Skip to content

Commit 981de07

Browse files
topointon-jumptopointon-jump
committed
flamenco, runtime, vm: clean up bounds on the number of instruction accounts
1 parent 9a344b1 commit 981de07

File tree

11 files changed

+112
-63
lines changed

11 files changed

+112
-63
lines changed

‎contrib/test/test-vectors-commit-sha.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
ebd84b9c0b327d78fefdc52236c3d91c3df2b946
1+
e3d072c318b9339379cac22d3278ca137220bfe4

‎src/flamenco/runtime/fd_runtime_const.h‎

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ static const fd_cluster_version_t FD_RUNTIME_CLUSTER_VERSION = {
105105
/* The bpf loader's serialization footprint is bounded in the worst case
106106
by 64 unique writable accounts which are each 10MiB in size (bounded
107107
by the amount of transaction accounts). We can also have up to
108-
FD_INSTR_ACCT_MAX (256) referenced accounts in an instruction.
108+
FD_BPF_INSTR_ACCT_MAX (255) referenced accounts in an instruction.
109109
110110
- 8 bytes for the account count
111111
For each account:
@@ -147,8 +147,63 @@ static const fd_cluster_version_t FD_RUNTIME_CLUSTER_VERSION = {
147147
#define FD_ACCOUNT_REC_ALIGN (8UL)
148148
/* https://github.com/anza-xyz/sbpf/blob/v0.12.2/src/ebpf.rs#L37-L38 */
149149
#define FD_RUNTIME_EBPF_HOST_ALIGN (16UL)
150-
#define FD_INSTR_ACCT_MAX (256)
151150

151+
/* FD_INSTR_ACCT_MAX is the maximum number of accounts that can
152+
be referenced by a single instruction.
153+
154+
This is different from FD_BPF_INSTR_ACCT_MAX, which is enforced by the
155+
BPF serializer. It is possible to pass in more than FD_BPF_INSTR_ACCT_MAX
156+
instruction accounts in a transaction (for example mainnet transaction)
157+
3eDdfZE6HswPxFKrtnQPsEmTkyL1iP57gRPEXwaqNGAqF1paGXCYYMwh7z4uQDUMgFor742sikVSQZW1gFRDhPNh).
158+
159+
A transaction like this will be loaded and sanitized, but will fail in the
160+
bpf serialization stage. It is also possible to invoke a native program with
161+
more than FD_BPF_INSTR_ACCT_MAX instruction accounts that will execute successfully.
162+
163+
Therefore we need to derive a bound from a worst-case transaction: one that
164+
has the maximum possible number of instruction accounts at the expense of
165+
everything else. This is a legacy transaction with a single account address,
166+
a single signature, a single instruction with empty data and as many
167+
instruction accounts as possible.
168+
169+
Therefore, the maximum number of instruction accounts is:
170+
(MTU - fixed overhead) / (size of instruction account)
171+
= (MTU
172+
- signature count (1 byte, value=1)
173+
- signature (64 bytes)
174+
- signature count in header (1 byte)
175+
- readonly signed count (1 byte)
176+
- readonly unsigned count (1 byte)
177+
- account count (1 byte, compact-u16 value=1)
178+
- 1 account address (32 bytes)
179+
- recent blockhash (32 bytes)
180+
- instruction count (1 byte, compact-u16 value=1)
181+
- program id index (1 byte)
182+
- instruction account count (2 bytes)
183+
- data len (1 byte, value=0)
184+
= 1232 - 1 - 64 - 1 - 1 - 1 - 1 - 32 - 32 - 1 - 1 - 2 - 1
185+
= 1094
186+
187+
TODO: SIMD-406 (https://github.com/solana-foundation/solana-improvement-documents/pull/406)
188+
limits the number of instruction accounts to 255 in transaction sanitization.
189+
190+
Once the corresponding feature gate has been activated, we can reduce
191+
FD_INSTR_ACCT_MAX to 255. We cannot reduce this before as this would cause
192+
the result of the get_processed_sibling_instruction syscall to diverge from
193+
Agave. */
194+
#define FD_INSTR_ACCT_MAX (1060UL)
195+
196+
/* FD_BPF_INSTR_ACCT_MAX is the maximum number of accounts that
197+
an instruction that goes through the bpf loader serializer can reference.
198+
199+
The BPF loader has a lower limit for the number of instruction accounts
200+
than is enforced in transaction sanitization.
201+
202+
TODO: remove this limit once SIMD-406 is activated, as we can then use the
203+
same limit everywhere.
204+
205+
https://github.com/anza-xyz/agave/blob/v3.1.4/transaction-context/src/lib.rs#L30-L32 */
206+
#define FD_BPF_INSTR_ACCT_MAX (255UL)
152207

153208
#define FD_BPF_LOADER_UNIQUE_ACCOUNT_FOOTPRINT(direct_mapping) \
154209
(1UL /* dup byte */ + \
@@ -168,7 +223,7 @@ static const fd_cluster_version_t FD_RUNTIME_CLUSTER_VERSION = {
168223
#define FD_BPF_LOADER_INPUT_REGION_FOOTPRINT(account_lock_limit, direct_mapping) \
169224
(FD_ULONG_ALIGN_UP( (sizeof(ulong) /* acct_cnt */ + \
170225
account_lock_limit*FD_BPF_LOADER_UNIQUE_ACCOUNT_FOOTPRINT(direct_mapping) + \
171-
(FD_INSTR_ACCT_MAX-account_lock_limit)*FD_BPF_LOADER_DUPLICATE_ACCOUNT_FOOTPRINT + \
226+
(FD_BPF_INSTR_ACCT_MAX-account_lock_limit)*FD_BPF_LOADER_DUPLICATE_ACCOUNT_FOOTPRINT + \
172227
sizeof(ulong) /* instr data len */ + \
173228
FD_TXN_MTU /* No instr data */ + \
174229
sizeof(fd_pubkey_t)), /* program id */ \

‎src/flamenco/runtime/info/fd_instr_info.c‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,14 @@ fd_instr_info_init_from_txn_instr( fd_instr_info_t * instr,
3333

3434
/* Set the program id */
3535
instr->program_id = txn_instr->program_id;
36-
37-
/* See note in fd_instr_info.h. TLDR: capping this value at 256
38-
should have literally 0 effect on program execution, down to the
39-
error codes. This is purely for the sake of not increasing the
40-
overall memory footprint of the transaction context. If this
41-
change causes issues, we may need to increase the array sizes in
42-
the instr info. */
43-
instr->acct_cnt = fd_ushort_min( txn_instr->acct_cnt, FD_INSTR_ACCT_MAX );
36+
instr->acct_cnt = txn_instr->acct_cnt;
37+
if( FD_UNLIKELY( instr->acct_cnt > FD_INSTR_ACCT_MAX ) ) {
38+
FD_LOG_CRIT(( "invariant violation: Instruction has too many accounts: %d > %lu", instr->acct_cnt, FD_INSTR_ACCT_MAX ));
39+
}
4440
instr->data_sz = txn_instr->data_sz;
4541
memcpy( instr->data, txn_in->txn->payload+txn_instr->data_off, instr->data_sz );
4642

47-
uchar acc_idx_seen[ FD_INSTR_ACCT_MAX ] = {0};
43+
uchar acc_idx_seen[ FD_TXN_ACCT_ADDR_MAX ] = {0};
4844

4945
for( ushort i=0; i<instr->acct_cnt; i++ ) {
5046
ushort acc_idx = instr_acc_idxs[i];

‎src/flamenco/runtime/info/fd_instr_info.h‎

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,6 @@
66
#include "../fd_runtime_const.h"
77
#include "../../../ballet/txn/fd_txn.h"
88

9-
/* While the maximum number of instruction accounts allowed for instruction
10-
execution is 256, it is entirely possible to have a transaction with more
11-
than 256 instruction accounts that passes transaction loading checks and enters
12-
`fd_execute_instr` (See mainnet transaction
13-
3eDdfZE6HswPxFKrtnQPsEmTkyL1iP57gRPEXwaqNGAqF1paGXCYYMwh7z4uQDUMgFor742sikVSQZW1gFRDhPNh
14-
for an example). An instruction that goes into the VM with more than 256 instruction accounts
15-
will fail, but you could also theoretically invoke a native program with over 256 random
16-
unreferenced instruction accounts that will execute successfully. The true bound for the
17-
maximum number of instruction accounts you can pass in is slighly lower than the maximum
18-
possible size for a serialized transaction (1232).
19-
20-
HOWEVER... to keep our memory footprint low, we cap the `acct_cnt` at 256 during setup since
21-
any extra accounts should (ideally) have literally 0 impact on program execution, whether
22-
or not they are present in the instr info. This keeps the transaction context size from
23-
blowing up to around 3MB in size. */
249
#define FD_INSTR_ACCT_FLAGS_IS_SIGNER (0x01U)
2510
#define FD_INSTR_ACCT_FLAGS_IS_WRITABLE (0x02U)
2611

@@ -79,7 +64,7 @@ fd_instruction_account_init( ushort idx_in_txn,
7964

8065
static inline void
8166
fd_instr_info_setup_instr_account( fd_instr_info_t * instr,
82-
uchar acc_idx_seen[ FD_INSTR_ACCT_MAX ],
67+
uchar acc_idx_seen[ FD_TXN_ACCT_ADDR_MAX ],
8368
ushort idx_in_txn,
8469
ushort idx_in_caller,
8570
ushort idx_in_callee,

‎src/flamenco/runtime/program/fd_bpf_loader_serialization.c‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,10 @@ fd_bpf_loader_input_serialize_aligned( fd_exec_instr_ctx_t * ctx,
266266
ulong * instr_data_offset ) {
267267
fd_pubkey_t * txn_accs = ctx->txn_out->accounts.account_keys;
268268

269-
uchar acc_idx_seen[ FD_INSTR_ACCT_MAX ] = {0};
270-
ushort dup_acc_idx[ FD_INSTR_ACCT_MAX ] = {0};
269+
/* Transaction sanitisation limits the number of instruction accounts to
270+
FD_TXN_ACCT_ADDR_MAX. */
271+
uchar acc_idx_seen[ FD_TXN_ACCT_ADDR_MAX ] = {0};
272+
ushort dup_acc_idx[ FD_TXN_ACCT_ADDR_MAX ] = {0};
271273

272274
/* 16-byte aligned buffer:
273275
https://github.com/anza-xyz/agave/blob/v3.0.0/program-runtime/src/serialization.rs#L60
@@ -550,8 +552,10 @@ fd_bpf_loader_input_serialize_unaligned( fd_exec_instr_ctx_t * ctx,
550552
ulong * instr_data_offset ) {
551553
fd_pubkey_t const * txn_accs = ctx->txn_out->accounts.account_keys;
552554

553-
uchar acc_idx_seen[FD_INSTR_ACCT_MAX] = {0};
554-
ushort dup_acc_idx[FD_INSTR_ACCT_MAX] = {0};
555+
/* Transaction sanitisation limits the number of instruction accounts to
556+
FD_TXN_ACCT_ADDR_MAX. */
557+
uchar acc_idx_seen[ FD_TXN_ACCT_ADDR_MAX ] = {0};
558+
ushort dup_acc_idx[ FD_TXN_ACCT_ADDR_MAX ] = {0};
555559

556560
/* 16-byte aligned buffer:
557561
https://github.com/anza-xyz/agave/blob/v2.2.13/programs/bpf_loader/src/serialization.rs#L32
@@ -761,7 +765,7 @@ fd_bpf_loader_input_serialize_parameters( fd_exec_instr_ctx_t * instr_ctx,
761765

762766
/* https://github.com/anza-xyz/agave/blob/v3.0.0/program-runtime/src/serialization.rs#L234-L237 */
763767
ulong num_ix_accounts = instr_ctx->instr->acct_cnt;
764-
if( FD_UNLIKELY( num_ix_accounts>=FD_INSTR_ACCT_MAX ) ) {
768+
if( FD_UNLIKELY( num_ix_accounts>FD_BPF_INSTR_ACCT_MAX ) ) {
765769
return FD_EXECUTOR_INSTR_ERR_MAX_ACCS_EXCEEDED;
766770
}
767771

‎src/flamenco/runtime/program/fd_native_cpi.c‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ fd_native_cpi_native_invoke( fd_exec_instr_ctx_t * ctx,
1515
ulong signers_cnt ) {
1616
/* Set up the instr info */
1717
fd_instr_info_t * instr_info = &ctx->runtime->instr.trace[ ctx->runtime->instr.trace_length++ ];
18-
fd_instruction_account_t instruction_accounts[ FD_INSTR_ACCT_MAX ];
18+
fd_instruction_account_t instruction_accounts[ FD_VM_CPI_MAX_INSTRUCTION_ACCOUNTS ];
1919
ulong instruction_accounts_cnt;
2020

2121
/* Set the stack size */
@@ -28,9 +28,8 @@ fd_native_cpi_native_invoke( fd_exec_instr_ctx_t * ctx,
2828
instr_info->program_id = (uchar)program_id;
2929
}
3030

31-
fd_pubkey_t instr_acct_keys[ FD_INSTR_ACCT_MAX ];
32-
uchar acc_idx_seen[ FD_INSTR_ACCT_MAX ];
33-
memset( acc_idx_seen, 0, FD_INSTR_ACCT_MAX );
31+
fd_pubkey_t instr_acct_keys[ FD_VM_CPI_MAX_INSTRUCTION_ACCOUNTS ];
32+
uchar acc_idx_seen[ FD_TXN_ACCT_ADDR_MAX ] = {0};
3433

3534
instr_info->acct_cnt = (ushort)acct_metas_len;
3635
for( ushort j=0U; j<acct_metas_len; j++ ) {

‎src/flamenco/runtime/tests/fd_instr_harness.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ fd_solfuzz_pb_instr_ctx_create( fd_solfuzz_runner_t * runner,
304304
ctx->sysvar_cache = fd_bank_sysvar_cache_modify( runner->bank );
305305
ctx->runtime = runtime;
306306

307-
uchar acc_idx_seen[ FD_INSTR_ACCT_MAX ] = {0};
307+
uchar acc_idx_seen[ FD_TXN_ACCT_ADDR_MAX ] = {0};
308308
for( ulong j=0UL; j < test_ctx->instr_accounts_count; j++ ) {
309309
uint index = test_ctx->instr_accounts[j].index;
310310
if( index >= test_ctx->accounts_count ) {

‎src/flamenco/vm/fd_vm_base.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,17 @@ FD_PROTOTYPES_END
247247

248248
#define FD_VM_MAX_CPI_INSTRUCTION_SIZE ( 1280UL) /* IPv6 Min MTU size */
249249

250+
/* FD_VM_CPI_MAX_INSTRUCTION_ACCOUNTS is the maximum number of accounts
251+
that can be referenced by a single CPI instruction.
252+
253+
Agave's bound for this is the same as their bound for the bound
254+
enforced by the bpf loader serializer.
255+
https://github.com/anza-xyz/agave/blob/v3.1.1/transaction-context/src/lib.rs#L32
256+
257+
TODO: when SIMD-406 is activated, we can use FD_INSTR_ACCT_MAX instead. */
258+
259+
#define FD_VM_CPI_MAX_INSTRUCTION_ACCOUNTS (FD_BPF_INSTR_ACCT_MAX)
260+
250261
/* FD_VM_CPI_BYTES_PER_UNIT is the number of account data bytes per
251262
compute unit charged during a cross-program invocation */
252263

‎src/flamenco/vm/syscall/fd_vm_syscall.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -775,8 +775,8 @@ int
775775
fd_vm_prepare_instruction( fd_instr_info_t * callee_instr,
776776
fd_exec_instr_ctx_t * instr_ctx,
777777
fd_pubkey_t const * callee_program_id_pubkey,
778-
fd_pubkey_t const instr_acct_keys[ FD_INSTR_ACCT_MAX ],
779-
fd_instruction_account_t instruction_accounts[ FD_INSTR_ACCT_MAX ],
778+
fd_pubkey_t const instr_acct_keys[ FD_VM_CPI_MAX_INSTRUCTION_ACCOUNTS ],
779+
fd_instruction_account_t instruction_accounts[ FD_VM_CPI_MAX_INSTRUCTION_ACCOUNTS ],
780780
ulong * instruction_accounts_cnt,
781781
fd_pubkey_t const * signers,
782782
ulong signers_cnt );

‎src/flamenco/vm/syscall/fd_vm_syscall_cpi.c‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,25 @@ int
5858
fd_vm_prepare_instruction( fd_instr_info_t * callee_instr,
5959
fd_exec_instr_ctx_t * instr_ctx,
6060
fd_pubkey_t const * callee_program_id_pubkey,
61-
fd_pubkey_t const instr_acct_keys[ FD_INSTR_ACCT_MAX ],
62-
fd_instruction_account_t instruction_accounts[ FD_INSTR_ACCT_MAX ],
61+
fd_pubkey_t const instr_acct_keys[ FD_VM_CPI_MAX_INSTRUCTION_ACCOUNTS ],
62+
fd_instruction_account_t instruction_accounts[ FD_VM_CPI_MAX_INSTRUCTION_ACCOUNTS ],
6363
ulong * instruction_accounts_cnt,
6464
fd_pubkey_t const * signers,
6565
ulong signers_cnt ) {
6666

6767
/* De-duplicate the instruction accounts, using the same logic as Solana */
6868
ulong deduplicated_instruction_accounts_cnt = 0;
69-
fd_instruction_account_t deduplicated_instruction_accounts[256] = {0};
70-
ulong duplicate_indicies_cnt = 0;
71-
ulong duplicate_indices[256] = {0};
69+
fd_instruction_account_t deduplicated_instruction_accounts[ FD_VM_CPI_MAX_INSTRUCTION_ACCOUNTS ] = {0};
70+
ulong duplicate_indicies_cnt = 0;
71+
ulong duplicate_indices[ FD_VM_CPI_MAX_INSTRUCTION_ACCOUNTS ] = {0};
72+
73+
/* This function is either called by a true CPI or by a native cpi invocation.
74+
The native CPI invocation is never called with more than 3 instruction
75+
accounts, and the true CPI is never called with more than
76+
FD_VM_CPI_MAX_INSTRUCTION_ACCOUNTS. */
77+
if( FD_UNLIKELY( callee_instr->acct_cnt > FD_VM_CPI_MAX_INSTRUCTION_ACCOUNTS ) ) {
78+
FD_LOG_CRIT(( "invariant violation: too many accounts %u", callee_instr->acct_cnt ));
79+
}
7280

7381
/* Normalize the privileges of each instruction account in the callee, after de-duping
7482
the account references.
@@ -237,15 +245,6 @@ get_cpi_max_account_infos( fd_bank_t * bank ) {
237245
return fd_ulong_if( FD_FEATURE_ACTIVE_BANK( bank, increase_tx_account_lock_limit ), FD_CPI_MAX_ACCOUNT_INFOS, 64UL );
238246
}
239247

240-
/* Maximum CPI instruction accounts. 255 was chosen to ensure that instruction
241-
accounts are always within the maximum instruction account limit for BPF
242-
program instructions.
243-
244-
https://github.com/solana-labs/solana/blob/dbf06e258ae418097049e845035d7d5502fe1327/sdk/program/src/syscalls/mod.rs#L19
245-
https://github.com/solana-labs/solana/blob/dbf06e258ae418097049e845035d7d5502fe1327/programs/bpf_loader/src/serialization.rs#L26 */
246-
247-
#define FD_CPI_MAX_INSTRUCTION_ACCOUNTS (255UL)
248-
249248
/* fd_vm_syscall_cpi_check_instruction contains common instruction acct
250249
count and data sz checks. Also consumes compute units proportional
251250
to instruction data size. */
@@ -256,7 +255,7 @@ fd_vm_syscall_cpi_check_instruction( fd_vm_t const * vm,
256255
ulong data_sz ) {
257256
/* https://github.com/anza-xyz/agave/blob/v3.1.2/program-runtime/src/cpi.rs#L146-L161 */
258257
if( FD_FEATURE_ACTIVE_BANK( vm->instr_ctx->bank, loosen_cpi_size_restriction ) ) {
259-
if( FD_UNLIKELY( acct_cnt > FD_CPI_MAX_INSTRUCTION_ACCOUNTS ) ) {
258+
if( FD_UNLIKELY( acct_cnt > FD_VM_CPI_MAX_INSTRUCTION_ACCOUNTS ) ) {
260259
// SyscallError::MaxInstructionAccountsExceeded
261260
return FD_VM_SYSCALL_ERR_MAX_INSTRUCTION_ACCOUNTS_EXCEEDED;
262261
}

0 commit comments

Comments
 (0)