Merge pull request #101 from metaplex-foundation/loris/allow-required…

…-accounts-after-optional-accounts Allow required accounts after optional accounts when rendering instruction keys
metaplex-foundation · Dec 15, 2022 · eb33811 · eb33811
2 parents 64514f2 + 530cfcb
commit eb33811
Show file tree

Hide file tree

Showing 3 changed files with 119 additions and 44 deletions.
diff --git a/.gitignore b/.gitignore
@@ -3,3 +3,4 @@ dist/
 yarn.lock
 test/integration/output
 .crates/
+node_modules
diff --git a/src/render-instruction.ts b/src/render-instruction.ts
@@ -11,7 +11,6 @@ import {
   PrimitiveTypeKey,
   isAccountsCollection,
 } from './types'
-import { strict as assert } from 'assert'
 import { ForceFixable, TypeMapper } from './type-mapper'
 import { renderDataStruct } from './serdes'
 import {
@@ -189,53 +188,22 @@ ${typeMapperImports.join('\n')}`.trim()
   private renderAccountKeysNotDefaultingOptionals(
     processedKeys: ProcessedAccountKey[]
   ) {
-    const requireds = processedKeys.filter((x) => !x.optional)
-    const optionals = processedKeys.filter((x, idx) => {
-      if (!x.optional) return false
-      assert(
-        idx >= requireds.length,
-        `All optional accounts need to follow required accounts, ${x.name} is not`
-      )
-      return true
-    })
+    const indexOfFirstOptional = processedKeys.findIndex((x) => x.optional)
+    if (indexOfFirstOptional === -1) {
+      return this.renderAccountKeysInsideArray(processedKeys) + '\n'
+    }
 
-    const requiredKeys = this.renderAccountKeysRequired(requireds)
-    const optionalKeys =
-      optionals.length > 0
-        ? optionals
-            .map(({ name, isMut, isSigner }, idx) => {
-              const requiredOptionals = optionals.slice(0, idx)
-              const requiredChecks = requiredOptionals
-                .map((x) => `accounts.${x.name} == null`)
-                .join(' || ')
-              const checkRequireds =
-                requiredChecks.length > 0
-                  ? `if (${requiredChecks}) { throw new Error('When providing \\'${name}\\' then ` +
-                    `${requiredOptionals
-                      .map((x) => `\\'accounts.${x.name}\\'`)
-                      .join(', ')} need(s) to be provided as well.') }`
-                  : ''
-              const pubkey = `accounts.${name}`
-              const accountMeta = renderAccountMeta(
-                pubkey,
-                isMut.toString(),
-                isSigner.toString()
-              )
-              // NOTE: we purposely don't add the default resolution here since the intent is to
-              // only pass that account when it is provided
-              return `
-  if (accounts.${name} != null) {
-    ${checkRequireds}
-    keys.push(${accountMeta})
-  }`
-            })
-            .join('\n') + '\n'
-        : ''
+    const accountsInsideArray = this.renderAccountKeysInsideArray(
+      processedKeys.slice(0, indexOfFirstOptional)
+    )
+    const accountsToPush = this.renderAccountKeysToPush(
+      processedKeys.slice(indexOfFirstOptional)
+    )
 
-    return `${requiredKeys}\n${optionalKeys}`
+    return `${accountsInsideArray}\n${accountsToPush}`
   }
 
-  private renderAccountKeysRequired(processedKeys: ProcessedAccountKey[]) {
+  private renderAccountKeysInsideArray(processedKeys: ProcessedAccountKey[]) {
     const metaElements = processedKeys
       .map((processedKey) =>
         renderRequiredAccountMeta(processedKey, this.programIdPubkey)
@@ -244,6 +212,55 @@ ${typeMapperImports.join('\n')}`.trim()
     return `[\n    ${metaElements}\n  ]`
   }
 
+  private renderAccountKeysToPush(processedKeys: ProcessedAccountKey[]) {
+    if (processedKeys.length === 0) {
+      return ''
+    }
+
+    const statements = processedKeys
+      .map((processedKey, idx) => {
+        if (!processedKey.optional) {
+          const accountMeta = renderRequiredAccountMeta(
+            processedKey,
+            this.programIdPubkey
+          )
+          return `keys.push(${accountMeta})`
+        }
+
+        const requiredOptionals = processedKeys
+          .slice(0, idx)
+          .filter((x) => x.optional)
+        const requiredChecks = requiredOptionals
+          .map((x) => `accounts.${x.name} == null`)
+          .join(' || ')
+        const checkRequireds =
+          requiredChecks.length > 0
+            ? `if (${requiredChecks}) { throw new Error('When providing \\'${processedKey.name}\\' then ` +
+              `${requiredOptionals
+                .map((x) => `\\'accounts.${x.name}\\'`)
+                .join(', ')} need(s) to be provided as well.') }`
+            : ''
+        const pubkey = `accounts.${processedKey.name}`
+        const accountMeta = renderAccountMeta(
+          pubkey,
+          processedKey.isMut.toString(),
+          processedKey.isSigner.toString()
+        )
+
+        // renderRequiredAccountMeta
+        // NOTE: we purposely don't add the default resolution here since the intent is to
+        // only pass that account when it is provided
+        return `
+if (accounts.${processedKey.name} != null) {
+  ${checkRequireds}
+  keys.push(${accountMeta})
+}`.trim()
+      })
+      .join('\n')
+
+    return `\n${statements}\n`
+  }
+
   // -----------------
   // AccountKeys: with strategy to defaultOptionalAccounts
   // -----------------

diff --git a/test/render-instruction.ts b/test/render-instruction.ts
@@ -239,6 +239,63 @@ test('ix: three accounts, two optional', async (t) => {
   })
 })
 
+test('ix: five accounts composed of two required, two optional and one required', async (t) => {
+  const ix = <IdlInstruction>{
+    name: 'sandwichedOptionalAccounts',
+    accounts: [
+      {
+        name: 'authority',
+        isMut: false,
+        isSigner: true,
+      },
+      {
+        name: 'metadata',
+        isMut: true,
+        isSigner: false,
+      },
+      {
+        name: 'useAuthorityRecord',
+        isMut: true,
+        isSigner: false,
+        desc: 'Use Authority Record PDA If present the program Assumes a delegated use authority',
+        optional: true,
+      },
+      {
+        name: 'burner',
+        isMut: false,
+        isSigner: false,
+        desc: 'Program As Signer (Burner)',
+        optional: true,
+      },
+      {
+        name: 'masterEdition',
+        isMut: false,
+        isSigner: false,
+      },
+    ],
+    args: [],
+  }
+  await checkRenderedIx(t, ix, [BEET_PACKAGE, SOLANA_WEB3_PACKAGE], {
+    rxs: [
+      // Ensuring that the pubkeys for optional accounts aren't required
+      /authority\: web3\.PublicKey/,
+      /metadata\: web3\.PublicKey/,
+      /useAuthorityRecord\?\: web3\.PublicKey/,
+      /burner\?\: web3\.PublicKey/,
+      /masterEdition\: web3\.PublicKey/,
+      // Ensuring we are pushing the last 3 accounts.
+      /keys\.push\(\{\s+pubkey\: accounts\.useAuthorityRecord,/,
+      /keys\.push\(\{\s+pubkey\: accounts\.burner,/,
+      /keys\.push\(\{\s+pubkey\: accounts\.masterEdition,/,
+    ],
+    nonrxs: [
+      // Ensuring we are not pushing the first 2 accounts.
+      /keys\.push\(\{\s+pubkey\: accounts\.authority,/,
+      /keys\.push\(\{\s+pubkey\: accounts\.metadata,/,
+    ]
+  })
+})
+
 test('ix: three accounts, two optional, defaultOptionalAccounts', async (t) => {
   const ix = <IdlInstruction>{
     name: 'choicy',