Skip to content
Merged
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
70 changes: 70 additions & 0 deletions src/commands/core/collection/add.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { baseUpdateAuthority, fetchAsset, fetchCollection, update } from '@metaplex-foundation/mpl-core'
import { publicKey } from '@metaplex-foundation/umi'
import { Args } from '@oclif/core'
import ora from 'ora'

import { generateExplorerUrl } from '../../../explorers.js'
import { TransactionCommand } from '../../../TransactionCommand.js'
import umiSendAndConfirmTransaction from '../../../lib/umi/sendAndConfirm.js'
import { txSignatureToString } from '../../../lib/util.js'

export default class CollectionAdd extends TransactionCommand<typeof CollectionAdd> {
static override description = 'Add an existing MPL Core Asset to a Collection'

static override examples = [
'<%= config.bin %> <%= command.id %> <collection> <asset>',
]

static override args = {
collection: Args.string({ description: 'Collection to add the asset to', required: true }),
asset: Args.string({ description: 'Asset to add to the collection', required: true }),
}

public async run(): Promise<unknown> {
const { args } = await this.parse(CollectionAdd)
const { umi, explorer, chain } = this.context

const spinner = ora('Fetching asset and collection...').start()

try {
const asset = await fetchAsset(umi, publicKey(args.asset))
const collection = await fetchCollection(umi, publicKey(args.collection))

Comment on lines +30 to +32
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.

🧹 Nitpick | 🔵 Trivial

Avoid unnecessary RPC on the early-reject path.

At Line 31, fetchCollection is called before checking whether the asset is already collection-bound (Line 33). If the asset is already in a collection, this does extra network work and latency for no benefit.

♻️ Proposed refactor
-      const asset = await fetchAsset(umi, publicKey(args.asset))
-      const collection = await fetchCollection(umi, publicKey(args.collection))
+      const asset = await fetchAsset(umi, publicKey(args.asset))
 
       if (asset.updateAuthority.type === 'Collection') {
         spinner.fail('Asset is already in a collection')
         this.error(`Asset ${args.asset} already belongs to collection ${asset.updateAuthority.address}`)
       }
+
+      const collection = await fetchCollection(umi, publicKey(args.collection))

Also applies to: 33-36

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/core/collection/add.ts` around lines 30 - 32, The code calls
fetchCollection before checking whether the asset is already collection-bound,
causing an unnecessary RPC; change the order in the add command so you first
fetch the asset with fetchAsset(publicKey(args.asset)) and check its collection
membership (e.g., asset.collection or equivalent) and only call
fetchCollection(publicKey(args.collection)) if the asset is not already in a
collection; ensure you still use publicKey(args.collection) when needed and keep
existing error/validation logic but move it after the early
collection-membership check to avoid unnecessary network calls.

if (asset.updateAuthority.type === 'Collection') {
spinner.fail('Asset is already in a collection')
this.error(`Asset ${args.asset} already belongs to collection ${asset.updateAuthority.address}`)
}

spinner.text = 'Adding asset to collection...'

const txBuilder = update(umi, {
asset,
newUpdateAuthority: baseUpdateAuthority('Collection', [collection.publicKey]),
newCollection: collection.publicKey,
})

const result = await umiSendAndConfirmTransaction(umi, txBuilder)
const signature = txSignatureToString(result.transaction.signature as Uint8Array)
const explorerUrl = generateExplorerUrl(explorer, chain, signature, 'transaction')

spinner.succeed(`Asset added to collection`)
this.logSuccess(`--------------------------------
Asset: ${args.asset}
Collection: ${args.collection}
Signature: ${signature}
Explorer: ${explorerUrl}
--------------------------------`)

return {
asset: args.asset,
collection: args.collection,
signature,
explorer: explorerUrl,
}
} catch (error) {
if (!spinner.isSpinning) throw error
spinner.fail('Failed to add asset to collection')
throw error
}
}
}
Loading