Add design proposal for ProgramInstruction procedural macro

docs/src/SUMMARY.md

@@ -117,3 +117,4 @@
   * [Block Confirmation](proposals/block-confirmation.md)
   * [Rust Clients](proposals/rust-clients.md)
   * [Optimistic Confirmation](proposals/optimistic_confirmation.md)
+  * [Program Instruction Macro](proposals/program-instruction-macro.md)

docs/src/proposals/program-instruction-macro.md

@@ -0,0 +1,224 @@
+# Program Instruction Macro
+
+## Problem
+
+Currently, inspecting an on-chain transaction requires depending on a
+client-side, language-specific decoding library to parse the instruction.  If
+rpc methods could return decoded instruction details, these custom solutions
+would be unnecessary.
+
+We can deserialize instruction data using a program's Instruction enum, but
+decoding the account-key list into human-readable identifiers requires manual
+parsing. Our current Instruction enums have that account information, but only
+in variant docs.
+
+Similarly, we have instruction constructor functions that duplicate nearly all
+the information in the enum, but we can't generate that constructor from the
+enum definition because the list of account references is in code comments.
+
+Also, Instruction docs can vary between implementations, as there is no
+mechanism to ensure consistency.
+
+## Proposed Solution
+
+Move the data from code comments to attributes, such that the constructors
+can be generated, and include all the documentation from the enum definition.
+
+Here is an example of an Instruction enum using the new accounts format:
+
+```rust,ignore
+#[instructions(test_program::id())]
+pub enum TestInstruction {
+    /// Transfer lamports
+    #[accounts(
+        from_account(signer, writable, desc = "Funding account"),
+        to_account(writable, desc = "Recipient account"),
+    )]
+    Transfer {
+        lamports: u64,
+    },
+
+    /// Provide M of N required signatures
+    #[accounts(
+        data_account(writable, desc = "Data account"),
+        signer(signer, multiple, desc = "Signer"),
+    )]
+    Multisig,
+
+    /// Consumes a stored nonce, replacing it with a successor
+    #[accounts(
+        nonce_account(signer, writable, desc = "Nonce account"),
+        recent_blockhashes_sysvar(signer, writable, desc = "RecentBlockhashes sysvar"),
+        nonce_authority(signer, optional, desc = "Nonce authority"),
+    )]
+    AdvanceNonceAccount,
+}
+```
+
+An example of the generated TestInstruction with docs:
+```rust,ignore
+    pub enum TestInstruction {
+      /// Transfer lamports
+      ///
+      /// * Accounts expected by this instruction:
+      ///   0. `[writable, signer]` Funding account
+      ///   1. `[writable]` Recipient account
+      Transfer {
+          lamports: u64,
+      },
+
+      /// Provide M of N required signatures
+      ///
+      /// * Accounts expected by this instruction:
+      ///   0. `[writable]` Data account
+      ///   * (Multiple) `[signer]` Signers
+      Multisig,
+
+      /// Consumes a stored nonce, replacing it with a successor
+      ///
+      /// * Accounts expected by this instruction:
+      ///   0. `[writable, signer]` Nonce account
+      ///   1. [] RecentBlockhashes sysvar
+      ///   2. (Optional) `[signer]` Nonce authority
+      AdvanceNonceAccount,
+  }
+```
+
+Generated constructors:
+```rust,ignore
+/// Transfer lamports
+///
+/// * `from_account` - `[writable, signer]` Funding account
+/// * `to_account` - `[writable]` Recipient account
+pub fn transfer(from_account: &Pubkey, to_account: &Pubkey, lamports: u64) -> Instruction {
+    let account_metas = vec![
+        AccountMeta::new(*from_pubkey, true),
+        AccountMeta::new(*to_pubkey, false),
+    ];
+    Instruction::new(
+        system_program::id(),
+        &SystemInstruction::Transfer { lamports },
+        account_metas,
+    )
+}
+
+/// Provide M of N required signatures
+///
+/// * `data_account` - `[writable]` Data account
+/// * `signers` - (Multiple) `[signer]` Signers
+pub fn multisig(data_account: &Pubkey, signers: &[&Pubkey]) -> Instruction {
+    let mut account_metas = vec![
+        AccountMeta::new(*nonce_pubkey, false),
+    ];
+    for pubkey in signers.iter() {
+        account_metas.push(AccountMeta::new_readonly(*pubkey, true));
+    }
+
+    Instruction::new(
+        test_program::id(),
+        &TestInstruction::Multisig,
+        account_metas,
+    )
+}
+
+/// Consumes a stored nonce, replacing it with a successor
+///
+/// * nonce_account - `[writable, signer]` Nonce account
+/// * recent_blockhashes_sysvar - [] RecentBlockhashes sysvar
+/// * nonce_authority - (Optional) `[signer]` Nonce authority
+pub fn advance_nonce_account(
+    nonce_account: &Pubkey,
+    recent_blockhashes_sysvar: &Pubkey,
+    nonce_authority: Option<&Pubkey>,
+) -> Instruction {
+    let mut account_metas = vec![
+        AccountMeta::new(*nonce_account, false),
+        AccountMeta::new_readonly(*recent_blockhashes_sysvar, false),
+    ];
+    if let Some(pubkey) = authorized_pubkey {
+        account_metas.push(AccountMeta::new_readonly(*nonce_authority, true));
+    }
+    Instruction::new(
+        test_program::id(),
+        &TestInstruction::AdvanceNonceAccount,
+        account_metas,
+    )
+}
+
+```
+
+Generated TestInstructionVerbose enum:
+
+```rust,ignore
+#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
+pub enum TestInstruction {
+    /// Transfer lamports
+    Transfer {
+        /// Funding account
+        funding_account: u8
+
+        /// Recipient account
+        recipient_account: u8
+
+        lamports: u64,
+    },
+
+    /// Provide M of N required signatures
+    Multisig {
+        data_account: u8,
+        signers: Vec<u8>,
+    },
+
+    /// Consumes a stored nonce, replacing it with a successor
+    AdvanceNonceAccount {
+        nonce_account: u8,
+        recent_blockhashes_sysvar: u8,
+        nonce_authority: Option<u8>,
+    }
+}
+
+impl TestInstructionVerbose {
+    pub fn from_instruction(instruction: TestInstruction, account_keys: Vec<u8>) -> Self {
+        match instruction {
+            TestInstruction::Transfer { lamports } => TestInstructionVerbose::Transfer {
+                funding_account: account_keys[0],
+                recipient_account: account_keys[1],
+                lamports,
+            }
+            TestInstruction::Multisig => TestInstructionVerbose::Multisig {
+                data_account: account_keys[0],
+                signers: account_keys[1..],
+            }
+            TestInstruction::AdvanceNonceAccount => TestInstructionVerbose::AdvanceNonceAccount {
+                nonce_account: account_keys[0],
+                recent_blockhashes_sysvar: account_keys[1],
+                nonce_authority: &account_keys.get(2),
+            }
+        }
+    }
+}
+
+```
+
+## Considerations
+
+1. **Named fields** - Since the resulting Verbose enum constructs variants with
+named fields, any unnamed fields in the original Instruction variant will need
+to have names generated. As such, it would be considerably more straightforward
+if all Instruction enum fields are converted to named types, instead of unnamed
+tuples. This seems worth doing anyway, adding more precision to the variants and
+enabling real documentation (so developers don't have to do
+[this](https://github.com/solana-labs/solana/blob/3aab13a1679ba2b7846d9ba39b04a52f2017d3e0/sdk/src/system_instruction.rs#L140)
+This will cause a little churn in our current code base, but not a lot.
+2. **Variable account lists** - This approach offers a couple options for
+variable account lists. First, optional accounts may be added and tagged with
+the `optional` keyword. However, currently only one optional account is
+supported per instruction. Additional data will need to be added to the
+instruction to support multiples, enabling identification of which accounts are
+present when some but not all are included. Second, accounts that share the same
+features may be added as a set, tagged with the `multiple` keyword. Like
+optional accounts, only one multiple account set is supported per instruction
+(and optional and multiple may not coexist). More complex instructions that
+cannot be accommodated by `optional` or `multiple`, requiring logic to figure
+out account order/representation, should probably be made into separate
+instructions.