Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions docs/sidebars.js
Original file line number Diff line number Diff line change
Expand Up @@ -213,22 +213,23 @@ module.exports = {
label: "Accepted",
items: [
"proposals/accepted-design-proposals",
"proposals/bankless-leader",
"proposals/block-confirmation",
"proposals/cluster-test-framework",
"proposals/embedding-move",
"proposals/interchain-transaction-verification",
"proposals/ledger-replication-to-implement",
"proposals/optimistic-confirmation-and-slashing",
"proposals/vote-signing-to-implement",
"proposals/cluster-test-framework",
"proposals/validator-proposal",
"proposals/optimistic_confirmation",
"proposals/rip-curl",
"proposals/rust-clients",
"proposals/simple-payment-and-state-verification",
"proposals/interchain-transaction-verification",
"proposals/snapshot-verification",
"proposals/bankless-leader",
"proposals/slashing",
"proposals/snapshot-verification",
"proposals/tick-verification",
"proposals/block-confirmation",
"proposals/rust-clients",
"proposals/optimistic_confirmation",
"proposals/embedding-move",
"proposals/rip-curl",
"proposals/transactions-v2",
"proposals/validator-proposal",
"proposals/vote-signing-to-implement",
],
},
],
Expand Down
41 changes: 38 additions & 3 deletions docs/src/proposals/transactions-v2.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,9 @@ pub struct MessageV2 {
// unchanged
pub recent_blockhash: Hash,

// unchanged. Account indices are still `u8` encoded so the max number of accounts
// in account_keys + address_maps is limited to 256.
/// Optimize instructions by separating dependent accounts from inputs
#[serde(with = "short_vec")]
pub instructions: Vec<CompiledInstruction>,
pub instructions: Vec<CompiledInstructionV2>,
}

#[derive(Serialize, Deserialize)]
Expand All @@ -158,15 +157,51 @@ pub struct AddressMap {
#[serde(with = "short_vec")]
pub entries: Vec<u8>,
}

#[derive(Serialize, Deserialize)]
pub struct CompiledInstructionV2 {
/// unchanged
pub program_id_index: u8,

/// unchanged
#[serde(with = "short_vec")]
pub accounts: Vec<u8>,

/// Only programs in this list may be invoked by the outer instruction
#[serde(with = "short_vec")]
pub inner_programs: Vec<u8>,

/// Dependent accounts do not need to be mapped to the memory of the outer
/// instruction's execution context. They are available for passing to inner
/// instructions
#[serde(with = "short_vec")]
pub inner_program_accounts: Vec<u8>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can avoid this additional churn to solve the problem described in this pr's description by providing yet another pathway (#17796 ) to access to tx's accounts in addition to the entrypoint one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can serve two purposes. One is avoiding the serialization overhead. The other (not mentioned in the PR description) is a stronger framework for isolating the scope of what an instruction can do. If we allow an instruction to dynamically load any account from the transaction, a malicious program might have enough resources for more damage. Imagine for instance, a program dynamically loads the fee payer signer account.


/// unchanged
#[serde(with = "short_vec")]
pub data: Vec<u8>,
}
```

#### Dependent accounts

The new transaction format supports lists of programs and accounts that a
transaction instruction may use when invoking inner instructions. These programs
and accounts are separate from transaction instruction account inputs so that
they do not need to incur the cost of serializing into the outer instruction's
memory which does need to read or write the account.

For extra user protection, only the programs listed in `inner_programs` may be
invoked by the outer transaction instruction.
Comment on lines +194 to +195
Copy link
Copy Markdown
Contributor

@ryoqun ryoqun Jun 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this strict requirement? is there any concrete threat in mind? if that's the case, I'll try to address this at #17796 as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I already outlined a general threat here: #17901 (comment) but more concretely: If a user inadvertently calls a compromised or otherwise malicious program in their transaction, sandboxing can help limit what damage may be done. Transactions can restrict which programs are called and which signers can be used in an instruction to avoid a program being able to invoke a system or token program CPI with a signer account.


#### Size changes

- 1 byte for `prefix` field
- 1 byte for version enum discriminant
- 1 byte for `address_maps` length
- Each map requires 2 bytes for `entries` length and `num_readonly`
- Each map entry is 1 byte (u8)
- Each instruction requires 2 bytes for the lengths of `inner_programs` and `inner_program_accounts`

#### Cost changes

Expand Down