feat: Universal NFT program on Solana #93
Conversation
…Call context structure
…o gateway call without SOL deposit
…fig structure, and improve error handling
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a new Solana Anchor "universal-nft" program with full NFT lifecycle and cross‑chain transfer support, plus account types, instruction contexts, events, utilities, CPIs, a TypeScript CLI example, Anchor/Cargo workspace configs, migration and package files, and shell automation scripts for devnet/testnet flows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as TS CLI
participant Program as Universal NFT (Anchor)
participant Meta as Metaplex Metadata
participant SPL as SPL Token
CLI->>Program: initialize(gateway_program, gateway_pda)
Program-->>CLI: ProgramConfigured (event)
CLI->>Program: reserve_next_token_id(mint)
Program-->>CLI: MintTicket created
CLI->>Program: mint_nft(name,symbol,uri)
Note right of Program: validate ticket → derive token_id & origin PDA
Program->>Meta: CreateMetadataAccountV3 (CPI)
Program->>Meta: CreateMasterEditionV3 (CPI)
Program->>SPL: MintTo amount=1 (CPI)
Program-->>CLI: NftMinted, OriginTracked
sequenceDiagram
autonumber
participant Owner as Owner/CLI
participant Program as Universal NFT (Anchor)
participant SPL as SPL Token
participant Gateway as ZetaChain Gateway
Owner->>Program: transfer_to_zetachain(token_id, zc_contract, dst_zrc20, recipient, deposit)
Note right of Program: validate origin, owner ATA, metadata
Program->>SPL: Burn NFT (CPI)
Program->>Gateway: deposit_and_call (CPI) with encoded payload
Program-->>Owner: CrossChainTransferInitiated, NftBurned
sequenceDiagram
autonumber
participant Gateway as ZetaChain Gateway
participant Program as Universal NFT (Anchor)
participant Meta as Metaplex
participant SPL as SPL Token
Gateway->>Program: on_call(amount,sender,data)
Note right of Program: decode payload, create/load origin, update state
Program->>Meta: CreateMetadata/CreateMasterEdition (CPI if needed)
Program->>SPL: MintTo program ATA (CPI)
Program-->>Gateway: NftMinted / success
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a comprehensive Universal NFT program on Solana using Anchor that enables cross-chain NFT transfers via ZetaChain Gateway integration. The implementation provides deterministic token ID generation, secure cross-chain operations, and complete metadata management.
Key changes:
- Implements a complete Universal NFT program with secure minting, burning, and cross-chain transfer capabilities
- Provides deterministic token ID generation using a two-step reserve-and-mint flow to prevent race conditions
- Integrates with ZetaChain Gateway for cross-chain operations with strict authentication and validation
Reviewed Changes
Copilot reviewed 35 out of 39 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib.rs | Main program implementation with core instructions (initialize, mint, transfer, on_call) |
| state/ | Account structures for config, NFT origin tracking, mint tickets, and metadata |
| util/ | Helper modules for constants, Metaplex integration, cross-chain data handling, and mint operations |
| context/ | Account validation contexts for each instruction |
| event/ | Event definitions for lifecycle, cross-chain, config, and origin tracking |
| error/ | Comprehensive error definitions |
| example/SolanaUniversalNFT.ts | TypeScript CLI client for interacting with the program |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
contracts/nft/contracts/solana/programs/universal-nft/src/util/constants.rs
Show resolved
Hide resolved
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (1)
Cargo.toml (1)
1-5: Duplication with the sub-workspace (contracts/nft/contracts/solana/Cargo.toml).There are two workspaces including the same program. See my earlier comment in contracts/nft/contracts/solana/Cargo.toml for consolidation options.
🧹 Nitpick comments (36)
contracts/nft/contracts/solana/programs/universal-nft/Xargo.toml (1)
1-2: Consider enabling panic_immediate_abort for Solana BPF builds.On Solana, aborting immediately on panic reduces binary size and avoids formatting code in panic paths. If you intend to keep Xargo, prefer enabling this std feature for the BPF target.
Apply this diff:
[target.bpfel-unknown-unknown.dependencies.std] -features = [] +features = ["panic_immediate_abort"]If you’re on the modern SBF toolchain where Xargo is no longer used in your build path, consider removing this file to avoid confusion.
contracts/nft/contracts/solana/.gitignore (1)
1-7: .gitignore looks fine for Anchor/Solana; minor Yarn v3 caveat.If you adopt Yarn Berry in this subproject, ignoring the entire .yarn directory will also ignore required files (releases/plugins). Otherwise, this is fine.
If you plan to use Yarn Berry, prefer:
-.yarn +.yarn/* +!.yarn/releases +!.yarn/plugins +!.yarn/sdks +!.yarn/versionscontracts/nft/contracts/solana/tsconfig.json (1)
2-9: Add Node types and modern lib for Anchor TS tests; set moduleResolution and skipLibCheck.Mocha/Anchor tests commonly reference Node globals (Buffer, process). Also, using a newer lib target avoids missing typings for modern JS. Adding skipLibCheck speeds up CI.
Apply this diff:
{ "compilerOptions": { - "types": ["mocha", "chai"], + "types": ["mocha", "chai", "node"], - "typeRoots": ["./node_modules/@types"], - "lib": ["es2015"], - "module": "commonjs", - "target": "es6", - "esModuleInterop": true + "typeRoots": ["./node_modules/@types"], + "lib": ["es2020"], + "module": "commonjs", + "moduleResolution": "node", + "target": "es2020", + "esModuleInterop": true, + "skipLibCheck": true } }contracts/nft/contracts/solana/Cargo.toml (2)
7-10: Consider adding opt-level="s" for Solana BPF release builds.For Solana programs, size and inlining matter. The common release profile is lto="fat", codegen-units=1, and opt-level="s" to keep the program within size limits while optimizing appropriately.
Apply this diff:
[profile.release] overflow-checks = true lto = "fat" +opt-level = "s" codegen-units = 1
1-6: Avoid maintaining nested Cargo workspaces unless there’s a strong reason.This repo defines a root workspace (Cargo.toml) and another workspace here that also includes the same program via programs/*. That can lead to drift in profiles/deps depending on where cargo is invoked (repo root vs this subdir). Prefer a single workspace root or document the dual-usage and keep settings in sync.
- Option A (simplest): Remove this sub-workspace and rely on the root workspace.
- Option B: Keep this sub-workspace but do not list the same packages in the root workspace members; treat this as an independent workspace used only when building from this subdir.
- Option C: Keep both, but centralize release profiles and dependency pins in one place and reference them consistently to avoid divergence.
contracts/nft/contracts/solana/migrations/deploy.ts (2)
7-12: Avoid mixing ESM imports with CommonJS exports in TS migration scripts.Anchor’s migration runner historically expects CommonJS. To prevent module interop pitfalls, keep both import and export in CJS style.
Apply this diff:
-import * as anchor from "@coral-xyz/anchor"; +// Use CJS style to match module.exports below and Anchor's migration loader +const anchor = require("@coral-xyz/anchor"); -module.exports = async function (provider: anchor.AnchorProvider) { +module.exports = async function (provider: any) { // Configure client to use the provider. anchor.setProvider(provider); // Add your deploy script here. };
9-11: Add a minimal sanity log to help operators confirm environment.Tiny, non-invasive logging helps validate the migration is running against the intended cluster/wallet.
Apply this diff:
// Configure client to use the provider. anchor.setProvider(provider); // Add your deploy script here. + const endpoint = provider.connection.rpcEndpoint; + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + const walletPubkey = provider.wallet?.publicKey?.toBase58?.() ?? "<unknown>"; + // Useful trace for CI and operators + console.log(`[universal-nft] Migration initialized. RPC: ${endpoint}, wallet: ${walletPubkey}`);contracts/nft/contracts/solana/programs/universal-nft/src/event/origin.rs (1)
13-17: “created_at” vs “timestamp” may be redundant; clarify or rename.Both fields are timestamps; consider either dropping one or renaming “timestamp” to “emitted_at” to avoid confusion between origin creation time and event emission time.
contracts/nft/contracts/solana/programs/universal-nft/src/error/mod.rs (3)
3-5: Stabilize error code ordering going forward.Anchor assigns error codes by enum order. After initial release, append new variants at the end only to keep codes stable across upgrades.
Apply this diff to document it in-code:
-#[error_code] +/// NOTE: Maintain variant order; append new variants at the end to preserve stable error codes. +#[error_code] pub enum UniversalNftError {
107-115: Disambiguate duplicate “Invalid metadata account” messages.Both InvalidMetadata and InvalidMetadataAccount have the same message, which is confusing during debugging/monitoring. Clarify the intent of each.
Apply this diff:
- #[msg("Invalid metadata account")] + #[msg("Invalid metadata PDA (Metaplex)")] InvalidMetadata, @@ - #[msg("Invalid metadata account")] + #[msg("Invalid metadata account (wrong owner or not PDA)")] InvalidMetadataAccount,Also applies to: 119-124
116-118: Categorization nit: CrossChainTransferFailed under METAPLEX ERRORS is misleading.Consider moving this variant to the CROSS-CHAIN ERRORS section for consistency. If you want to avoid reordering now, add a comment and relocate later before the first public release.
contracts/nft/contracts/solana/Anchor.toml (1)
17-22: Consider adding a test validator config to clone required external programsYour program CPIs into Metaplex Token Metadata (and possibly the Gateway). Local tests often need those programs’ code cloned into the local test validator, or they’ll fail at CPI time.
You can extend Anchor.toml like this (choose the right Gateway program address your tests use):
[provider] cluster = "devnet" wallet = "~/.config/solana/id.json" [scripts] test = "yarn run ts-mocha -p ./tsconfig.json -t 1000000 tests/**/*.ts" + +[test.validator] +# Option: clone from devnet so the local validator has these programs available during `anchor test` +url = "https://api.devnet.solana.com" +clone = [ + # Metaplex Token Metadata + { address = "metaqbxxUerdq28cj1RbAWkYQm3ybzjb6a8bt518x1s" }, + # SPL Associated Token (if needed) + { address = "ATokenGPvbdGVxr1b2hvZbsiqW5xWH25efTNsLJA8knL" }, + # ZetaChain Gateway program (replace with the address you pin in config) + # { address = "<GATEWAY_PROGRAM_ADDRESS>" }, +]If your test suite spins up its own validator or runs on devnet, this may be unnecessary; otherwise, it saves a lot of CPI headaches.
contracts/nft/contracts/solana/programs/universal-nft/src/event/mod.rs (1)
1-4: Nit: add a module-level doc to aid discoverabilityA short comment about what each submodule covers helps future contributors.
Apply:
+//! Event definitions for the Universal NFT program, organized by concern. +//! - nft_lifecycle: mint/burn lifecycle events +//! - cross_chain: cross-chain transfer events +//! - config: program configuration events +//! - origin: NFT origin tracking events pub mod nft_lifecycle; pub mod cross_chain; pub mod config; pub mod origin;contracts/nft/contracts/solana/programs/universal-nft/src/context/update_config.rs (1)
5-16: Add brief context docs; consider pause semantics (optional)
- Add a short docstring to explain what fields can be updated and who may call it.
- If your design requires that updates remain possible while
is_paused, ensure the instruction handler doesn’t block updates when paused (context is fine; this is about handler logic).You can add docs:
#[derive(Accounts)] pub struct UpdateConfig<'info> { + /// Program config PDA. Only the `authority` can update fields. + /// Note: Updates may be allowed regardless of `is_paused` (enforced in handler). #[account( mut, seeds = [UNIVERSAL_NFT_CONFIG_SEED], bump = config.bump, has_one = authority )] pub config: Account<'info, UniversalNftConfig>, pub authority: Signer<'info>, }contracts/nft/contracts/solana/programs/universal-nft/src/event/nft_lifecycle.rs (1)
20-32: Optional: include a hex-encoded token_id for human-readable logsIndexers can convert bytes to hex, but adding a precomputed hex string can simplify observability and debugging, especially across chains.
If you want this, consider emitting both fields:
#[event] pub struct NftMinted { pub mint: Pubkey, pub token_id: [u8; 32], + /// Hex-encoded token ID (for human-readable logs) + pub token_id_hex: String, pub owner: Pubkey, pub uri: String, pub timestamp: i64, } #[event] pub struct NftBurned { pub mint: Pubkey, pub token_id: [u8; 32], + /// Hex-encoded token ID (for human-readable logs) + pub token_id_hex: String, pub owner: Pubkey, pub timestamp: i64, pub reason: String, }This is optional—only if you value log readability over a few extra bytes.
contracts/nft/contracts/solana/programs/universal-nft/Cargo.toml (1)
20-23: Pinning Anchor deps is fine; consider workspace unification and metadata (optional)
- If you already use
[workspace.dependencies], consider referencing them here to avoid drift across crates.- Add
licenseandrepositoryfor completeness if publishing/artifact tracking matters.Example:
[package] name = "universal-nft" version = "0.1.0" description = "Created with Anchor" edition = "2021" +license = "Apache-2.0" +repository = "https://github.com/<org>/<repo>" [dependencies] -anchor-lang = "0.31.1" -anchor-spl = "0.31.1" +anchor-lang = { workspace = true } +anchor-spl = { workspace = true } hex = "0.4"contracts/nft/contracts/solana/programs/universal-nft/src/util/mint_helpers.rs (2)
7-9: Document preconditions and invariants for safe NFT mintingClarify what must hold true for callers (mint authority, mint/account match, decimals==0). This prevents subtle misuse across contexts.
-/// Mint an NFT token to a recipient's token account -/// This is a common operation used in mint_nft, on_call, and on_revert +/// Mint an NFT token to a recipient's token account. +/// Common operation used in mint_nft, on_call, and on_revert. +/// Preconditions: +/// - token_account.mint == mint_account.key() +/// - mint has decimals == 0 (enforced by caller/context) +/// - `authority` is the mint's mint_authority (or a PDA with `authority_signer_seeds`) +/// - amount is fixed to 1 by design (NFT semantics)
15-35: Reduce duplication and add a defensive mint/mint-account check
- Build MintTo accounts once and branch only on signer usage.
- Add a debug assertion to catch mismatched token account mint during development.
) -> Result<()> { - let mint_to_ctx = if let Some(signer_seeds) = authority_signer_seeds { - CpiContext::new_with_signer( - token_program.to_account_info(), - MintTo { - mint: mint_account.to_account_info(), - to: token_account.to_account_info(), - authority: authority.clone(), - }, - signer_seeds, - ) - } else { - CpiContext::new( - token_program.to_account_info(), - MintTo { - mint: mint_account.to_account_info(), - to: token_account.to_account_info(), - authority: authority.clone(), - }, - ) - }; + debug_assert_eq!( + token_account.mint, + mint_account.key(), + "token_account.mint does not match provided mint" + ); + let accounts = MintTo { + mint: mint_account.to_account_info(), + to: token_account.to_account_info(), + authority: authority.clone(), + }; + let mint_to_ctx = match authority_signer_seeds { + Some(seeds) => CpiContext::new_with_signer( + token_program.to_account_info(), + accounts, + seeds, + ), + None => CpiContext::new(token_program.to_account_info(), accounts), + };contracts/nft/contracts/solana/programs/universal-nft/src/event/config.rs (1)
3-34: LGTM: Clear and appropriately scoped config eventsThe event shapes look good for initialize/update flows and include the essential provenance fields. If you intend Option in ProgramConfigUpdated to signal “unchanged” vs “changed to None”, consider documenting that semantic in the context modules for log consumers.
contracts/nft/contracts/solana/programs/universal-nft/src/context/reserve_next_token_id.rs (1)
25-26: Optional: add a defensive constraint on mintSince mint is only used as a seed, this is fine. If you want extra safety, consider asserting it’s a system-owned account (i.e., not already initialized by an unexpected program) when reserving:
- constraint = mint.owner == system_program.key()
Not strictly required for correctness.
- /// CHECK: only used as seed - pub mint: UncheckedAccount<'info>, + /// CHECK: only used as seed + #[account(constraint = mint.owner == system_program.key())] + pub mint: UncheckedAccount<'info>,contracts/nft/contracts/solana/programs/universal-nft/src/context/initialize.rs (1)
19-21: Optional: move gateway executability check into account constraintsYou already validate executability in the handler. You can push it into account constraints for earlier failure.
- /// CHECK: ZetaChain gateway program (must be executable) - pub gateway_program: UncheckedAccount<'info>, + /// CHECK: ZetaChain gateway program (must be executable) + #[account(constraint = gateway_program.executable @ crate::error::UniversalNftError::InvalidGatewayProgram)] + pub gateway_program: UncheckedAccount<'info>,contracts/nft/contracts/solana/programs/universal-nft/src/util/cross_chain_helpers.rs (3)
80-104: Add encode_nft_data_with_recipient to complement the decode variantYou expose a decode variant with a recipient but only provide an encode variant without it. Adding a symmetric encoder reduces duplication at call sites and avoids error-prone manual concatenation.
Apply this diff to add the missing encoder:
@@ pub fn encode_nft_data( @@ data } +/// Encode NFT data (with recipient) for cross-chain message +/// Format: token_id(32) + source_chain(8) + recipient(32) + metadata_uri + name + symbol +pub fn encode_nft_data_with_recipient( + token_id: [u8; 32], + source_chain: u64, + recipient: [u8; 32], + metadata_uri: &str, + name: &str, + symbol: &str, +) -> Vec<u8> { + let mut data = Vec::new(); + // Token ID (32 bytes) + data.extend_from_slice(&token_id); + // Source chain (8 bytes, big-endian) + data.extend_from_slice(&source_chain.to_be_bytes()); + // Recipient (32 bytes) + data.extend_from_slice(&recipient); + // Strings (each length-prefixed with 4-byte big-endian length) + encode_length_prefixed_string(&mut data, metadata_uri); + encode_length_prefixed_string(&mut data, name); + encode_length_prefixed_string(&mut data, symbol); + data +}
145-151: Normalize 0x/0X prefix and trim whitespace in hex parsingCurrently only lowercase "0x" is recognized, and leading/trailing whitespace will cause a failure. Minor hardening below.
-pub fn hex_string_to_address(address_str: &str) -> Result<[u8; 20]> { - let hex_str = if address_str.starts_with("0x") { - &address_str[2..] - } else { - address_str - }; +pub fn hex_string_to_address(address_str: &str) -> Result<[u8; 20]> { + let addr = address_str.trim(); + let hex_str = if addr.starts_with("0x") || addr.starts_with("0X") { + &addr[2..] + } else { + addr + };
105-129: Consider enforcing length caps for decoded stringsProgram-wide limits are documented (capped name/symbol/uri). Enforcing them here would fail fast on malformed/crafted payloads and reduce allocation risk.
If you have constants (e.g., MAX_NAME_LEN, MAX_SYMBOL_LEN, MAX_URI_LEN) in util/constants, check lengths after extraction and return InvalidDataFormat on overflow. I can draft a patch once constants are confirmed.
contracts/nft/contracts/solana/programs/universal-nft/src/event/cross_chain.rs (1)
62-74: Add token and chain context to CrossChainTransferRevertedThe revert event currently lacks token_id and chain IDs, making it hard to correlate with the original transfer. Add parity fields for observability.
#[event] pub struct CrossChainTransferReverted { + /// Universal token ID being reverted + pub token_id: [u8; 32], + /// Source chain ID + pub source_chain: u64, + /// Destination chain ID + pub destination_chain: u64, /// Original sender who initiated the transfer pub sender: Pubkey, /// Amount of SOL refunded (if any) pub amount: u64, /// Reason for the revert pub revert_reason: String, /// Timestamp of revert pub timestamp: i64, /// Whether an NFT was restored during revert pub nft_restored: bool, }This will require updating emit sites accordingly.
contracts/nft/contracts/solana/programs/universal-nft/src/context/mint_nft.rs (3)
47-57: Add owner constraints for Metadata PDAEven though the PDA derivation is constrained to TOKEN_METADATA_PROGRAM_ID via seeds::program, adding an explicit owner check is a simple hardening step.
#[account( mut, seeds = [ b"metadata", TOKEN_METADATA_PROGRAM_ID.as_ref(), mint.key().as_ref(), ], bump, + owner = TOKEN_METADATA_PROGRAM_ID, seeds::program = TOKEN_METADATA_PROGRAM_ID )] pub metadata: UncheckedAccount<'info>,
61-72: Add owner constraints for Master Edition PDASame rationale as metadata.
#[account( mut, seeds = [ b"metadata", TOKEN_METADATA_PROGRAM_ID.as_ref(), mint.key().as_ref(), b"edition", ], bump, + owner = TOKEN_METADATA_PROGRAM_ID, seeds::program = TOKEN_METADATA_PROGRAM_ID )] pub master_edition: UncheckedAccount<'info>,
74-82: Use init_if_needed for ATA to allow idempotent mints to the same recipientIf the recipient’s ATA already exists (e.g., retries or pre-existing account), init will fail. init_if_needed improves UX without weakening constraints.
#[account( - init, + init_if_needed, payer = payer, associated_token::mint = mint, associated_token::authority = recipient, )] pub token_account: Account<'info, TokenAccount>,contracts/nft/contracts/example/SolanaUniversalNFT.ts (1)
18-28: Remove dead helpers or wire them where neededtoU64LE and sha256 are currently unused, increasing noise. Either remove them or hook them up where intended.
-function toU64LE(n: bigint): Buffer { - const b = Buffer.alloc(8); - b.writeBigUInt64LE(n); - return b; -} - -function sha256(...parts: Buffer[]): Buffer { - const h = crypto.createHash("sha256"); - for (const p of parts) h.update(p); - return h.digest(); -} +// Helpers toU64LE and sha256 removed (unused). Reintroduce when needed.contracts/nft/contracts/solana/programs/universal-nft/src/context/on_call.rs (2)
61-68: Constrain metadata_program with explicit address match error from your error enumCurrently this uses Anchor’s generic ConstraintAddress error. For consistency and clearer diagnostics, return your domain error (e.g., InvalidMetadataProgram) or reuse InvalidGatewayProgram analog for metadata.
- #[account(constraint = metadata_program.key() == TOKEN_METADATA_PROGRAM_ID @ anchor_lang::error::ErrorCode::ConstraintAddress)] + #[account( + constraint = metadata_program.key() == TOKEN_METADATA_PROGRAM_ID + @ crate::error::UniversalNftError::InvalidMetadata + )]
20-24: Consider allowing ATA auto-creation or documenting requirementon_call assumes pda_ata exists and only validates derivation. If the gateway invokes this CPI without an existing ATA, mint will fail. Either:
- add optional associated token program account and create ATA when missing; or
- document that the caller must pre-create pda_ata.
I can provide a minimal ATA-create branch guarded by data_is_empty() and presence of associated token program in remaining_accounts.
Also applies to: 43-57
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (2)
469-497: on_call funds metadata/edition creation from the config PDA; ensure it’s funded or accept an external payerYou use the config PDA as payer (with signer seeds) for metadata and master edition CPIs, but no guarantee exists that this PDA has enough lamports. Consider:
- accepting a payer account in OnCall context; or
- ensuring the gateway pre-funds the config PDA; or
- failing early with a clear error if balance is insufficient.
I can add a balance check with Rent::minimum_balance and a dedicated error to surface this condition.
Also applies to: 558-566, 567-581
21-21: Remove dead code commentLeftover commented import // use std::io::Seek; can be removed.
-// use std::io::Seek;contracts/nft/contracts/solana/programs/universal-nft/src/context/transfer_to_zetachain.rs (2)
41-52: Redundant constraint on metadata if seeds verify Metaplex PDAYou both seed-derive metadata via TOKEN_METADATA_PROGRAM_ID seeds and assert equality to nft_origin.original_metadata. Either is sufficient; keeping both is fine but adds CU and complexity. Consider keeping only the PDA derivation, which already guarantees the correct address for this mint.
- constraint = metadata.key() == nft_origin.original_metadata @ crate::error::UniversalNftError::InvalidMetadata,
57-64: Gateway program validation good; ensure executable is enforced at Initialize time onlyYou already enforce executable at initialize; duplicating executable check here is harmless. If CU-sensitive, you can rely on the earlier guarantee and keep only key equality.
Also applies to: 65-71
contracts/nft/contracts/solana/programs/universal-nft/src/util/constants.rs (1)
109-114: Hardcoded ZetaChain gateway IDs: consider environment gatingYou’ve embedded devnet and mainnet IDs as comments with a devnet default constant. Suggest exposing this via Initialize (which you do) and keep constants only for documentation, or guard with cfg(feature = "...") so binaries don’t accidentally reference the wrong cluster.
I can refactor to remove this constant and rely solely on program config set in initialize/update_config, reducing risk of mismatched IDs.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (4)
contracts/nft/contracts/solana/Cargo.lockis excluded by!**/*.lockcontracts/nft/contracts/solana/yarn.lockis excluded by!**/yarn.lock,!**/*.lockcontracts/nft/package-lock.jsonis excluded by!**/package-lock.jsoncontracts/nft/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (35)
Cargo.toml(1 hunks)contracts/nft/contracts/example/SolanaUniversalNFT.ts(1 hunks)contracts/nft/contracts/solana/.gitignore(1 hunks)contracts/nft/contracts/solana/.prettierignore(1 hunks)contracts/nft/contracts/solana/Anchor.toml(1 hunks)contracts/nft/contracts/solana/Cargo.toml(1 hunks)contracts/nft/contracts/solana/migrations/deploy.ts(1 hunks)contracts/nft/contracts/solana/package.json(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/Cargo.toml(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/Xargo.toml(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/context/initialize.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/context/mint_nft.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/context/mod.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/context/on_call.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/context/reserve_next_token_id.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/context/transfer_to_zetachain.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/context/update_config.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/error/mod.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/event/config.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/event/cross_chain.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/event/mod.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/event/nft_lifecycle.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/event/origin.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/state/config.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/state/metadata.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/state/mint_ticket.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/state/mod.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/util/constants.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/util/cross_chain_helpers.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/util/metaplex_helpers.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/util/mint_helpers.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/util/mod.rs(1 hunks)contracts/nft/contracts/solana/tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
contracts/nft/contracts/solana/programs/universal-nft/src/context/mod.rs (2)
contracts/nft/contracts/example/SolanaUniversalNFT.ts (1)
initialize(94-108)contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (6)
initialize(29-67)mint_nft(103-241)transfer_to_zetachain(245-397)on_call(399-592)update_config(595-656)reserve_next_token_id(71-99)
contracts/nft/contracts/solana/programs/universal-nft/src/event/mod.rs (1)
contracts/nft/contracts/example/SolanaUniversalNFT.ts (1)
origin(217-227)
contracts/nft/contracts/solana/programs/universal-nft/src/context/reserve_next_token_id.rs (1)
contracts/nft/contracts/example/SolanaUniversalNFT.ts (1)
mint(110-161)
contracts/nft/contracts/solana/programs/universal-nft/src/util/cross_chain_helpers.rs (1)
contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs (1)
new(31-49)
contracts/nft/contracts/solana/programs/universal-nft/src/util/metaplex_helpers.rs (1)
contracts/nft/contracts/example/SolanaUniversalNFT.ts (1)
mint(110-161)
contracts/nft/contracts/solana/programs/universal-nft/src/context/mint_nft.rs (1)
contracts/nft/contracts/example/SolanaUniversalNFT.ts (1)
mint(110-161)
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (5)
contracts/nft/contracts/solana/programs/universal-nft/src/util/metaplex_helpers.rs (2)
create_metadata_account(11-92)create_master_edition_account(95-159)contracts/nft/contracts/solana/programs/universal-nft/src/util/cross_chain_helpers.rs (1)
decode_nft_data_with_recipient(44-78)contracts/nft/contracts/solana/programs/universal-nft/src/util/mint_helpers.rs (1)
mint_nft_to_recipient(9-39)contracts/nft/contracts/example/SolanaUniversalNFT.ts (2)
initialize(94-108)mint(110-161)contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs (1)
new(31-49)
contracts/nft/contracts/solana/programs/universal-nft/src/context/transfer_to_zetachain.rs (1)
contracts/nft/contracts/example/SolanaUniversalNFT.ts (1)
mint(110-161)
🔇 Additional comments (26)
contracts/nft/contracts/solana/.prettierignore (1)
1-7: Prettier ignore set is appropriate for this workspace.Covers common build and local artifacts for Anchor/Solana projects.
contracts/nft/contracts/solana/programs/universal-nft/src/event/origin.rs (1)
5-18: URI length is already constrained before emission
Theuriparameter passed intoOriginTrackedcomes from the user‐supplieduriinmint_nft, which is gated by:// src/lib.rs:109–113 require!(name.len() <= MAX_NAME_LENGTH, UniversalNftError::NameTooLong); require!(symbol.len() <= MAX_SYMBOL_LENGTH, UniversalNftError::SymbolTooLong); require!(uri.len() <= MAX_URI_LENGTH, UniversalNftError::UriTooLong);Because
original_uri(and the cross‐chainmetadata_uri) both derive directly from this validateduri, no additional length checks are needed before emitting the event.Cargo.toml (2)
7-9: Centralizing Anchor deps looks good.Pinning anchor-lang and anchor-spl at the workspace level reduces version skew across crates.
7-9: Ensure matching Solana SDK 2.1.0 and anchor-cli 0.31.1To avoid build failures with Anchor 0.31.1, confirm both your local and CI toolchains use the following versions:
• Solana SDK: v2.1.0 (Agave-renamed binaries)
• anchor-cli: v0.31.1Recommended installation via AVM:
avm install 0.31.1 avm use 0.31.1Update your CI workflows and developer setup docs to enforce these versions.
contracts/nft/contracts/solana/Anchor.toml (1)
8-12: Program ID alignment verified
Thedeclare_id!incontracts/nft/contracts/solana/programs/universal-nft/src/lib.rsmatches both thelocalnetanddevnetentries incontracts/nft/contracts/solana/Anchor.toml. No further changes needed.contracts/nft/contracts/solana/programs/universal-nft/src/event/mod.rs (1)
1-9: Module organization and re-exports look goodClear separation by concern and flattened re-exports for ergonomic use in
lib.rs. No issues spotted.contracts/nft/contracts/solana/programs/universal-nft/src/context/update_config.rs (1)
7-16: Account constraints are correct and minimal
- PDA is derived from a constant seed with
bump = config.bump.has_one = authorityties the config’s authority to the provided signer.- No unnecessary accounts pulled in.
Assuming
UniversalNftConfigcontains anauthority: Pubkeyandbump: u8, this is good.contracts/nft/contracts/solana/programs/universal-nft/src/event/nft_lifecycle.rs (1)
5-18: NftMinted event shape is clear and usefulFields provide enough context for off-chain indexers. Using
[u8; 32]fortoken_idkeeps it compact.contracts/nft/contracts/solana/programs/universal-nft/Cargo.toml (1)
11-18: Feature flags are sensible for Anchor workflowsThe set (
cpi,no-entrypoint,no-idl, etc.) aligns with common patterns. No issues spotted.contracts/nft/contracts/solana/programs/universal-nft/src/state/mod.rs (1)
1-9: Public re-exports are consistent and conciseModule layout and re-exports are clean, forming a stable public API surface for state types.
contracts/nft/contracts/solana/programs/universal-nft/src/state/config.rs (1)
27-29: UNIVERSAL_NFT_CONFIG_SPACE matches struct size and is properly documentedVerified that in
contracts/nft/contracts/solana/programs/universal-nft/src/util/constants.rs
the comment and value ofUNIVERSAL_NFT_CONFIG_SPACE(8 + 32 + 32 + 32 + 8 + 8 + 1 + 8 + 1 = 130) exactly correspond to the fields ofUniversalNftConfig(122 bytes) plus the 8-byte discriminator. No further action needed.contracts/nft/contracts/solana/programs/universal-nft/src/context/reserve_next_token_id.rs (2)
7-14: Config PDA constraints look solidSeeded PDA with has_one authority and explicit bump usage are correct and align with the program’s deterministic config design.
16-23: Ensure the MintTicket is closed post-mint to refund rentTicket init looks correct here. Given the lib.rs comment that the ticket will be “auto-closed to authority via the close attribute,” please verify that MintNft context declares:
- #[account(mut, close = authority)] for ticket
Without the close attribute in MintNft, rent will remain locked.
Would you like me to propose the exact MintNft context attribute changes?
contracts/nft/contracts/solana/programs/universal-nft/src/util/mod.rs (1)
1-9: LGTM: clean module surface and re-exportsThe module plumbing is clear and promotes a unified util surface.
contracts/nft/contracts/solana/programs/universal-nft/src/context/initialize.rs (2)
24-27: Good: enforce gateway_pda ownership by the provided programThis prevents spoofing the gateway PDA and aligns with handler-level checks.
8-14: UNIVERSAL_NFT_CONFIG_SPACE already includes the 8-byte discriminator
I’ve confirmed inutil/constants.rs(line 26) thatpub const UNIVERSAL_NFT_CONFIG_SPACE: usize = 8 + 32 + 32 + 32 + 8 + 8 + 1 + 8 + 1;includes the initial
8‐byte discriminator. No changes needed here.contracts/nft/contracts/solana/programs/universal-nft/src/context/mod.rs (1)
1-13: LGTM: context modules and re-exports are consistentClear split of contexts and public re-export surface. No issues.
contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs (2)
31-49: Constructor looks correctInitial values (updated_at = created_at, is_on_solana = true) and bump storage are appropriate.
51-62: LGTM: state transition helpers are minimal and clearThe mark_transferred_off_solana and mark_to_solana helpers keep invariants simple and explicit.
contracts/nft/contracts/solana/programs/universal-nft/src/util/cross_chain_helpers.rs (1)
10-39: Bounds-checked parsing looks solidThe minimal-length guard and subsequent string extraction with bounds checks are correct and avoid panics on malformed data.
contracts/nft/contracts/solana/programs/universal-nft/src/event/cross_chain.rs (1)
4-24: Event definitions read wellField types and layout are appropriate for logging transfer initiation with sufficient context (ids, sender, recipient, gas_limit, nonce).
contracts/nft/contracts/solana/programs/universal-nft/src/state/metadata.rs (1)
3-20: LGTM: Metaplex-compatible metadata structsTypes mirror the common Metaplex shapes and derive the right Anchor traits for (de)serialization.
contracts/nft/contracts/solana/programs/universal-nft/src/context/mint_nft.rs (1)
30-35: Handler must strictly verify nft_origin PDA derivationYou’ve noted this as CHECK. Ensure the handler recomputes the token_id from ticket data, derives the expected PDA, and rejects mismatches before any state changes.
If helpful, I can generate a patch for the handler to enforce: derive -> compare -> initialize -> write.
contracts/nft/contracts/example/SolanaUniversalNFT.ts (1)
94-108: Initialize: verify gatewayPda matches program expectationThe on-chain program pins both gateway_program and gateway_pda. Consider validating gatewayPda client-side early (format/public key) and surfacing a clearer error if mismatch occurs.
I can add a preflight that fetches config (if exists) and warns on mismatch before submitting the tx.
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (1)
332-370: Verify ZetaChain Gateway instruction format and IDL presence
I couldn’t find an Anchor IDL or JSON schema for the ZetaChain gateway in the repository. Before shipping, please:
- Confirm whether the gateway program is Anchor-based and locate its IDL (e.g. in @zetachain/protocol-contracts-solana).
- If it is, add the IDL as a dependency and derive the 8-byte “call” discriminator from it instead of hard-coding
sha256("global:call")[..8].- Ensure the
accounts: Vec<AccountMeta>you supply (payer, gateway PDA, signer, system program, etc.) matches exactly what the gateway’scallexpects—many gateway CPIs also require an explicit payer or additional accounts.- If the gateway is not Anchor-based, update the instruction data packing to match its documented wire format rather than Anchor’s Borsh-discriminator layout.
contracts/nft/contracts/solana/programs/universal-nft/src/util/constants.rs (1)
83-94: Ensure DEFAULT_ flags align with your Metaplex CPI usage*You define DEFAULT_PRIMARY_SALE_HAPPENED and DEFAULT_IS_MUTABLE but only use DEFAULT_SELLER_FEE_BASIS_POINTS. When switching to mpl-token-metadata builders, consider whether you want to set is_mutable=true and whether to expose primary_sale_happened logic (usually handled by token-metadata program flows).
Do you plan to toggle primary_sale_happened at any point? If so, we can wire that into the metadata update flow.
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs
Outdated
Show resolved
Hide resolved
contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs
Show resolved
Hide resolved
contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs
Show resolved
Hide resolved
contracts/nft/contracts/solana/programs/universal-nft/src/util/metaplex_helpers.rs
Outdated
Show resolved
Hide resolved
contracts/nft/contracts/solana/programs/universal-nft/src/util/metaplex_helpers.rs
Show resolved
Hide resolved
contracts/nft/contracts/solana/programs/universal-nft/src/util/metaplex_helpers.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
contracts/nft/contracts/solana/programs/universal-nft/src/state/mod.rs (2)
5-7: Replace wildcard re-exports with explicit items (+ optional prelude)Avoid
pub use ...::*to prevent unintentionally exposing helpers/constants and to keep a stable, intentional API surface. Re-export only the account types consumers need and optionally provide apreludefor ergonomic imports.Apply this diff:
-pub use config::*; -pub use nft_origin::*; -pub use mint_ticket::*; +pub use self::config::UniversalNftConfig; +pub use self::nft_origin::NftOrigin; +pub use self::mint_ticket::MintTicket; + +/// Convenient import group for consumers: +/// use crate::state::prelude::*; +pub mod prelude { + pub use super::{MintTicket, NftOrigin, UniversalNftConfig}; +}If more items from these modules are intended for external use, we can explicitly list them here as needed rather than glob-exporting.
1-3: Privatize submodules and expose a stable APINo direct uses of
state::config::,state::nft_origin::orstate::mint_ticket::were found (ranrg -nP --type=rust -C2 '\bstate::(config|nft_origin|mint_ticket)::'). You can safely hide these modules and rely on existing re-exports.• File:
contracts/nft/contracts/solana/programs/universal-nft/src/state/mod.rs• Diff to apply:
+//! State module for the Universal NFT program. //! Submodules are kept private; public items are re-exported below. -pub mod config; -pub mod nft_origin; -pub mod mint_ticket; +mod config; +mod nft_origin; +mod mint_ticket;This optional refactor minimizes your public surface and reduces API-churn risk. Let me know if you need any help updating imports or re-exports.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
contracts/nft/contracts/solana/programs/universal-nft/src/state/mod.rs(1 hunks)
…token program and simplify metadata account creation
…nhance TypeScript configuration
…eation using builder pattern
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
contracts/nft/contracts/solana/programs/universal-nft/src/util/metaplex_helpers.rs (2)
50-74: Fix CPI account order: missing duplicate authority and wrong ordering for CreateMetadataAccountV3The accounts passed to invoke/invoke_signed must match the builder’s metas (including duplicates for the same pubkey). Current slices omit the second authority (update_authority) and shift system/rent into the wrong positions, which will cause the CPI to fail at runtime.
Align the AccountInfo slices with the builder order: [metadata, mint, mint_authority, payer, update_authority, system_program, rent].
Apply this diff:
@@ - match authority_signer_seeds { - Some(seeds) => anchor_lang::solana_program::program::invoke_signed( - &ix, - &[ - metadata.clone(), - mint.clone(), - authority.clone(), - payer.clone(), - system_program.clone(), - rent.clone(), - ], - seeds, - ).map_err(|_| UniversalNftError::MetadataCreationFailed)?, - None => invoke( - &ix, - &[ - metadata.clone(), - mint.clone(), - authority.clone(), - payer.clone(), - system_program.clone(), - rent.clone(), - ], - ).map_err(|_| UniversalNftError::MetadataCreationFailed)?, - } + match authority_signer_seeds { + Some(seeds) => anchor_lang::solana_program::program::invoke_signed( + &ix, + &[ + metadata.clone(), // metadata + mint.clone(), // mint + authority.clone(), // mint_authority + payer.clone(), // payer + authority.clone(), // update_authority (same key, duplicated) + system_program.clone(), // system_program + rent.clone(), // rent + ], + seeds, + ).map_err(|_| UniversalNftError::MetadataCreationFailed)?, + None => invoke( + &ix, + &[ + metadata.clone(), // metadata + mint.clone(), // mint + authority.clone(), // mint_authority + payer.clone(), // payer + authority.clone(), // update_authority (same key, duplicated) + system_program.clone(), // system_program + rent.clone(), // rent + ], + ).map_err(|_| UniversalNftError::MetadataCreationFailed)?, + }
92-104: Fix CPI account order and max_supply type for CreateMasterEditionV3Two issues:
- Account slice must match builder metas and include both update_authority and mint_authority (even if same key). Current order omits one authority and shifts subsequent accounts.
- Builder .max_supply expects Option (None for unlimited, Some(0) to disallow prints). Passing 0u64 is likely a type mismatch depending on mpl-token-metadata version.
Align with builder order: [edition, mint, update_authority, mint_authority, payer, metadata, token_program, system_program, rent] and set max_supply to Some(0) for a 1/1 NFT with no prints.
Apply this diff:
@@ - let ix = CreateMasterEditionV3Builder::new() + let ix = CreateMasterEditionV3Builder::new() .edition(*master_edition.key) .mint(*mint.key) .update_authority(*authority.key) .mint_authority(*authority.key) .payer(*payer.key) .metadata(*metadata.key) .token_program(*token_program.key) .system_program(*system_program.key) .rent(Some(*rent.key)) - .max_supply(0u64) + .max_supply(Some(0u64)) .instruction(); @@ - Some(seeds) => anchor_lang::solana_program::program::invoke_signed( + Some(seeds) => anchor_lang::solana_program::program::invoke_signed( &ix, &[ - master_edition.clone(), - mint.clone(), - authority.clone(), - payer.clone(), - metadata.clone(), - token_program.clone(), - system_program.clone(), - rent.clone(), + master_edition.clone(), // edition + mint.clone(), // mint + authority.clone(), // update_authority + authority.clone(), // mint_authority (same key, duplicated) + payer.clone(), // payer + metadata.clone(), // metadata + token_program.clone(), // token_program + system_program.clone(), // system_program + rent.clone(), // rent ], seeds, ).map_err(|_| UniversalNftError::MasterEditionCreationFailed)?, None => invoke( &ix, &[ - master_edition.clone(), - mint.clone(), - authority.clone(), - payer.clone(), - metadata.clone(), - token_program.clone(), - system_program.clone(), - rent.clone(), + master_edition.clone(), // edition + mint.clone(), // mint + authority.clone(), // update_authority + authority.clone(), // mint_authority (same key, duplicated) + payer.clone(), // payer + metadata.clone(), // metadata + token_program.clone(), // token_program + system_program.clone(), // system_program + rent.clone(), // rent ], ).map_err(|_| UniversalNftError::MasterEditionCreationFailed)?,Also applies to: 105-133
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (1)
178-194: Allocate NftOrigin with discriminator space (8 bytes) to avoid OOB writesYou allocate only NftOrigin::LEN but later write discriminator + data (8 + LEN), which will overrun the buffer.
Apply this diff:
- let rent = Rent::get()?; - let space = NftOrigin::LEN as u64; - let lamports = rent.minimum_balance(space as usize); + let rent = Rent::get()?; + let space = (8 + NftOrigin::LEN) as u64; + let lamports = rent.minimum_balance(space as usize);
🧹 Nitpick comments (1)
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (1)
129-144: Idempotency of NftOrigin creation vs. PR objectiveThe PR summary claims NftOrigin is created idempotently. Here, if the account already exists and is owned by this program, you error with NftAlreadyExists. Consider making this idempotent by verifying the existing data matches the intended values and then proceeding without error.
I can draft a patch that loads existing NftOrigin, validates it matches (token_id, mint, metadata, uri), and no-ops if so.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
contracts/nft/contracts/example/SolanaUniversalNFT.ts(1 hunks)contracts/nft/contracts/solana/package.json(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/Cargo.toml(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/util/metaplex_helpers.rs(1 hunks)contracts/nft/contracts/solana/tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- contracts/nft/contracts/solana/tsconfig.json
- contracts/nft/contracts/solana/package.json
- contracts/nft/contracts/solana/programs/universal-nft/Cargo.toml
🧰 Additional context used
🧬 Code Graph Analysis (1)
contracts/nft/contracts/solana/programs/universal-nft/src/util/metaplex_helpers.rs (1)
contracts/nft/contracts/example/SolanaUniversalNFT.ts (1)
mint(110-161)
🔇 Additional comments (4)
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (2)
203-219: Copy length assumes discriminator + data; ensure allocation fix appliesThis region copies 8 + serialized_len bytes into the account buffer. With the allocation fix above, this write becomes safe. Without it, it corrupts memory and fails at runtime.
After applying the allocation change at Lines 178-194, re-run tests or a local integration to ensure account creation + write succeeds for mint_nft.
331-359: Manually encoded Gateway.call payload: verify ABI and field order against the Gateway IDLThe discriminant and Borsh encodings look plausible, but this is brittle. If Gateway’s IDL changes (field order or types), this CPI will break. Prefer encoding via the Gateway’s Anchor IDL/program client or verify against the canonical IDL.
Would you like me to wire in the Gateway IDL and generate the instruction data using Anchor’s coder to avoid manual packing?
contracts/nft/contracts/example/SolanaUniversalNFT.ts (2)
163-174: Good client-side validation for destinationChain and finalRecipientThe added checks align with on-chain constraints (u64 range, recipient length ≤ 128) and will improve UX before RPC calls.
136-155: Ensure accountsStrict matches updated on-chain contextmintNft accounts include associatedTokenProgram and metadataProgram. The on-chain helpers use the TOKEN_METADATA_PROGRAM_ID constant and don’t need metadataProgram directly; associatedTokenProgram is only required if the program/Accounts init_if_needed creates the ATA.
Confirm that the on-chain MintNft context expects both associatedTokenProgram and metadataProgram in its accounts list; otherwise accountsStrict will reject these extras. I can adjust the CLI or the program Accounts definition once you confirm the IDL.
…nstants for NftOrigin, UniversalNftConfig, and MintTicket
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (2)
146-157: Master edition CPI now correctly passes the SPL Token program — good catchBoth call sites use token_program for CreateMasterEditionV3 as required by the updated helper signature.
Also applies to: 484-496
178-201: NftOrigin account sizing looks correct given NFT_ORIGIN_SPACE includes the discriminatorAllocations use NftOrigin::SPACE, and serialization writes discriminator + data. This matches the constant that already accounts for the 8-byte discriminator per prior clarification.
Also applies to: 517-543
🧹 Nitpick comments (3)
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (3)
206-218: Minor: trim vector capacity to avoid confusionVec::with_capacity(8 + NftOrigin::LEN) over-allocates. Capacity = NftOrigin::LEN is sufficient since you write exactly discriminator(8) + struct bytes (= LEN total).
- let mut tmp = Vec::with_capacity(8 + NftOrigin::LEN); + let mut tmp = Vec::with_capacity(NftOrigin::LEN);and similarly in the on_call path:
- let mut tmp = Vec::with_capacity(8 + NftOrigin::LEN); + let mut tmp = Vec::with_capacity(NftOrigin::LEN);Also applies to: 548-555
614-637: Optional hardening: require presence of new gateway accounts when updating configupdate_config optionally validates the provided new_gateway_program/new_gateway_pda if the accounts are present. To keep the gateway pin strong, consider requiring the corresponding accounts in remaining_accounts whenever those fields are updated, and validate executability/ownership deterministically.
Example approach:
- If new_gateway_program.is_some() → require the program account be present and executable.
- If new_gateway_pda.is_some() → require the account be present and owned by the (possibly updated) gateway program.
398-406: Naming nit: ctx.accounts.pda is the config account — consider renaming for clarityIn this context, pda is the UniversalNftConfig PDA. Renaming it to config (and keeping a separate config_pda bump if needed) will reduce cognitive load.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
contracts/nft/contracts/solana/programs/universal-nft/src/context/initialize.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/context/reserve_next_token_id.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/state/config.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/state/mint_ticket.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/util/constants.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs
- contracts/nft/contracts/solana/programs/universal-nft/src/state/mint_ticket.rs
- contracts/nft/contracts/solana/programs/universal-nft/src/context/initialize.rs
- contracts/nft/contracts/solana/programs/universal-nft/src/context/reserve_next_token_id.rs
- contracts/nft/contracts/solana/programs/universal-nft/src/state/config.rs
- contracts/nft/contracts/solana/programs/universal-nft/src/util/constants.rs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: alperimo
PR: zeta-chain/standard-contracts#93
File: contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs:178-194
Timestamp: 2025-08-22T12:08:58.785Z
Learning: In the Solana universal-nft program, NFT_ORIGIN_SPACE constant already includes the 8-byte Anchor discriminator in its calculation (8 + 32 + 32 + 32 + 4 + MAX_URI_LENGTH + 8 + 8 + 1 + 1), and NftOrigin::LEN correctly uses this value for account space allocation.
📚 Learning: 2025-08-22T12:08:58.785Z
Learnt from: alperimo
PR: zeta-chain/standard-contracts#93
File: contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs:178-194
Timestamp: 2025-08-22T12:08:58.785Z
Learning: In the Solana universal-nft program, NFT_ORIGIN_SPACE constant already includes the 8-byte Anchor discriminator in its calculation (8 + 32 + 32 + 32 + 4 + MAX_URI_LENGTH + 8 + 8 + 1 + 1), and NftOrigin::LEN correctly uses this value for account space allocation.
Applied to files:
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs
🧬 Code graph analysis (1)
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (4)
contracts/nft/contracts/solana/programs/universal-nft/src/util/metaplex_helpers.rs (2)
create_metadata_account(15-77)create_master_edition_account(80-136)contracts/nft/contracts/solana/programs/universal-nft/src/util/cross_chain_helpers.rs (1)
decode_nft_data_with_recipient(44-78)contracts/nft/contracts/solana/programs/universal-nft/src/util/mint_helpers.rs (1)
mint_nft_to_recipient(9-39)contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs (1)
new(40-58)
🔇 Additional comments (3)
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (3)
268-277: ATA checks are solidStrict ATA derivation and key equality checks for owner and mint are correctly enforced prior to burning.
132-157: Mint flow sequencing and early ticket-use are correctMarking the MintTicket as used before side effects, followed by metadata and master edition creation, reduces double-spend risk. Good supply and decimals checks, too.
33-44: Gateway pinning during initialize is robustYou validate the gateway program’s key and executability and pin the gateway_pda by owner — aligns with the stated security model.
| let current_ix = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative( | ||
| -1, | ||
| ix_account, | ||
| ).map_err(|_| UniversalNftError::InvalidCaller)?; | ||
|
|
||
| require!( | ||
| current_ix.program_id == ctx.accounts.pda.gateway_program, | ||
| UniversalNftError::InvalidCaller | ||
| ); | ||
|
|
||
| // Optionally assert gateway PDA owner | ||
| require!( | ||
| *ctx.accounts.gateway_pda.owner == ctx.accounts.pda.gateway_program, | ||
| UniversalNftError::InvalidCaller | ||
| ); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix gateway caller verification: use current instruction (0), also assert the pinned gateway PDA is present
Using get_instruction_relative(-1) checks the previous top-level instruction, not the current one executing your CPI. An attacker can place a benign gateway instruction earlier in the transaction and then CPI into on_call from a different program to bypass this check. Verify the current instruction (0) and ensure it includes the pinned gateway_pda from config.
Run this to find any other unsafe uses:
Proposed patch:
- let current_ix = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative(
- -1,
- ix_account,
- ).map_err(|_| UniversalNftError::InvalidCaller)?;
-
- require!(
- current_ix.program_id == ctx.accounts.pda.gateway_program,
- UniversalNftError::InvalidCaller
- );
+ // IMPORTANT: When this program is invoked via CPI by the gateway, the "current"
+ // top-level instruction is the gateway instruction itself.
+ let current_ix = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative(
+ 0,
+ ix_account,
+ ).map_err(|_| UniversalNftError::InvalidCaller)?;
+
+ require_keys_eq!(
+ current_ix.program_id,
+ ctx.accounts.pda.gateway_program,
+ UniversalNftError::InvalidCaller
+ );
+
+ // Ensure the pinned gateway PDA is included among the accounts of the invoking instruction.
+ let pinned_gw_pda = ctx.accounts.pda.gateway_pda;
+ let gw_pda_present = current_ix.accounts.iter().any(|meta| meta.pubkey == pinned_gw_pda);
+ require!(gw_pda_present, UniversalNftError::InvalidCaller);
// Optionally assert gateway PDA owner
require!(
*ctx.accounts.gateway_pda.owner == ctx.accounts.pda.gateway_program,
UniversalNftError::InvalidCaller
);🏁 Script executed:
#!/bin/bash
rg -nP -C2 'get_instruction_relative\s*\(\s*-1' --type=rustLength of output: 61
🏁 Script executed:
#!/bin/bash
# Search for occurrences of ix_account in Rust files to verify the referenced sysvar account is correct
rg -n "ix_account" --type=rust -C2
# Verify any uses of get_instruction_relative (with any offset) to ensure no other unsafe calls remain
rg -nP "get_instruction_relative" --type=rust -C2Length of output: 1717
Fix gateway caller verification: use current instruction (0) and assert pinned gateway PDA presence
The call to get_instruction_relative(-1) reads the previous top-level instruction, allowing an attacker to “preload” a gateway call earlier in the transaction and then CPI into on_call from a different program to bypass the check. Instead, inspect the current instruction (0) and ensure the pinned gateway PDA from your config is actually one of its accounts.
• Location to update:
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rsaround lines 416–431
Proposed patch:
- let current_ix = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative(
- -1,
- ix_account,
- ).map_err(|_| UniversalNftError::InvalidCaller)?;
-
- require!(
- current_ix.program_id == ctx.accounts.pda.gateway_program,
- UniversalNftError::InvalidCaller
- );
+ // IMPORTANT: use offset 0 to inspect the *current* top-level instruction.
+ let current_ix = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative(
+ 0,
+ ix_account,
+ ).map_err(|_| UniversalNftError::InvalidCaller)?;
+
+ // Ensure it’s issued by the configured gateway program
+ require_keys_eq!(
+ current_ix.program_id,
+ ctx.accounts.pda.gateway_program,
+ UniversalNftError::InvalidCaller
+ );
+
+ // And verify the pinned gateway PDA is among its account metas
+ let pinned_gw = ctx.accounts.pda.gateway_pda;
+ let has_pinned = current_ix
+ .accounts
+ .iter()
+ .any(|meta| meta.pubkey == pinned_gw);
+ require!(has_pinned, UniversalNftError::InvalidCaller);
// Optionally assert that the gateway PDA’s owner matches the gateway program
require!(
*ctx.accounts.gateway_pda.owner == ctx.accounts.pda.gateway_program,
UniversalNftError::InvalidCaller
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let current_ix = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative( | |
| -1, | |
| ix_account, | |
| ).map_err(|_| UniversalNftError::InvalidCaller)?; | |
| require!( | |
| current_ix.program_id == ctx.accounts.pda.gateway_program, | |
| UniversalNftError::InvalidCaller | |
| ); | |
| // Optionally assert gateway PDA owner | |
| require!( | |
| *ctx.accounts.gateway_pda.owner == ctx.accounts.pda.gateway_program, | |
| UniversalNftError::InvalidCaller | |
| ); | |
| // IMPORTANT: use offset 0 to inspect the *current* top-level instruction. | |
| let current_ix = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative( | |
| 0, | |
| ix_account, | |
| ).map_err(|_| UniversalNftError::InvalidCaller)?; | |
| // Ensure it’s issued by the configured gateway program | |
| require_keys_eq!( | |
| current_ix.program_id, | |
| ctx.accounts.pda.gateway_program, | |
| UniversalNftError::InvalidCaller | |
| ); | |
| // And verify the pinned gateway PDA is among its account metas | |
| let pinned_gw = ctx.accounts.pda.gateway_pda; | |
| let has_pinned = current_ix | |
| .accounts | |
| .iter() | |
| .any(|meta| meta.pubkey == pinned_gw); | |
| require!(has_pinned, UniversalNftError::InvalidCaller); | |
| // Optionally assert that the gateway PDA’s owner matches the gateway program | |
| require!( | |
| *ctx.accounts.gateway_pda.owner == ctx.accounts.pda.gateway_program, | |
| UniversalNftError::InvalidCaller | |
| ); |
🤖 Prompt for AI Agents
In contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs around lines
416–431, the code reads the previous top-level instruction with
get_instruction_relative(-1) which allows an attacker to preload a gateway call;
change to get_instruction_relative(0) to inspect the current instruction, keep
the program_id == ctx.accounts.pda.gateway_program check, and add an explicit
assertion that the pinned gateway PDA (ctx.accounts.gateway_pda.key()) is
present among the current instruction's account metas (iterate
current_ix.accounts or check account list for the PDA pubkey) so the gateway PDA
from config is actually one of the current instruction accounts before
proceeding.
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs
Outdated
Show resolved
Hide resolved
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs
Outdated
Show resolved
Hide resolved
…ding messages expected by UniversalNFTCore.onCall on ZEVM
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (3)
478-492: Fix gateway caller verification in on_call: use current instruction (0) and assert pinned Gateway PDA is presentUsing get_instruction_relative(-1) checks the previous top-level instruction. An attacker can front-load a benign gateway instruction and then CPI into on_call from a different program to bypass this check. Verify the current instruction (0) instead, and also ensure the pinned gateway PDA from config is one of the current instruction’s account metas.
Apply this diff:
- let current_ix = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative( - -1, - ix_account, - ).map_err(|_| UniversalNftError::InvalidCaller)?; - - require!( - current_ix.program_id == ctx.accounts.pda.gateway_program, - UniversalNftError::InvalidCaller - ); + // IMPORTANT: inspect the current top-level instruction (offset 0) + let current_ix = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative( + 0, + ix_account, + ).map_err(|_| UniversalNftError::InvalidCaller)?; + + require_keys_eq!( + current_ix.program_id, + ctx.accounts.pda.gateway_program, + UniversalNftError::InvalidCaller + ); + + // Ensure the pinned gateway PDA is present among the current instruction's accounts + let pinned_gw_pda = ctx.accounts.pda.gateway_pda; + let gw_pda_present = current_ix.accounts.iter().any(|meta| meta.pubkey == pinned_gw_pda); + require!(gw_pda_present, UniversalNftError::InvalidCaller);Run to ensure there are no other unsafe uses:
#!/bin/bash # Find any other uses of get_instruction_relative with negative offsets rg -nP --type=rust -C2 'get_instruction_relative\s*\(\s*-\d+\s*,'
282-287: Verify the provided gateway_pda equals the pinned one in config (not just owned by gateway program)Ownership-only checks allow any account owned by the gateway program to be passed. Require exact key equality with the pinned config.gateway_pda to prevent misrouting.
Apply this diff:
require!( *ctx.accounts.gateway_pda.owner == ctx.accounts.config.gateway_program, UniversalNftError::InvalidGatewayProgram ); + require_keys_eq!( + ctx.accounts.gateway_pda.key(), + ctx.accounts.config.gateway_pda, + UniversalNftError::InvalidGatewayProgram + );
617-625: Make on_call mint idempotent and enforce supply invariantson_call always mints 1, which can inflate supply if retried or called twice while the token is already on Solana. Enforce decimals==0 and supply/balance invariants; mint only when supply is 0, otherwise assert the invariant state.
Apply this diff:
- // 3) Mint 1 to program_ata using config PDA as authority - mint_nft_to_recipient( - &ctx.accounts.mint_account, - &ctx.accounts.pda_ata, - &ctx.accounts.pda.to_account_info(), - &ctx.accounts.token_program, - Some(signer_seeds), - )?; + // 3) Enforce supply invariants and mint idempotently to program_ata + require!(ctx.accounts.mint_account.decimals == 0, UniversalNftError::InvalidMint); + if ctx.accounts.mint_account.supply == 0 { + mint_nft_to_recipient( + &ctx.accounts.mint_account, + &ctx.accounts.pda_ata, + &ctx.accounts.pda.to_account_info(), + &ctx.accounts.token_program, + Some(signer_seeds), + )?; + } else { + // Already minted to Solana: enforce idempotency invariants + require!(ctx.accounts.mint_account.supply == 1, UniversalNftError::InvalidTokenSupply); + require!(ctx.accounts.pda_ata.amount == 1, UniversalNftError::InvalidTokenAmount); + }
🧹 Nitpick comments (17)
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (4)
494-496: Enforce inbound length caps in on_call for parity with mint_nftDecode accepts untrusted strings (name, symbol, metadata_uri). Mirror the MAX_* caps used in mint_nft to keep invariants consistent and avoid oversized inputs.
Apply this diff:
- let (token_id, origin_chain, _recipient_bytes, metadata_uri, name, symbol) = decode_nft_data_with_recipient(&data)?; + let (token_id, origin_chain, _recipient_bytes, metadata_uri, name, symbol) = + decode_nft_data_with_recipient(&data)?; + require!(name.len() <= MAX_NAME_LENGTH, UniversalNftError::NameTooLong); + require!(symbol.len() <= MAX_SYMBOL_LENGTH, UniversalNftError::SymbolTooLong); + require!(metadata_uri.len() <= MAX_URI_LENGTH, UniversalNftError::UriTooLong);
370-422: Gate verbose CPI diagnostics behind a feature flag (or remove before shipping)The large hex/log blocks are great for debugging but add compute/log noise and leak payloads. Gate them behind #[cfg(feature = "verbose-cpi-logs")] or remove.
Suggested wrap (condensed for brevity):
- // TODO: Remove: Diagnostics: Print everything needed for the CPI invoke --- - // Hex helpers - let to_hex = |bytes: &[u8]| -> String { - let mut s = String::with_capacity(bytes.len() * 2); - for b in bytes { - use core::fmt::Write as _; - let _ = write!(&mut s, "{:02x}", b); - } - s - }; - let receiver_hex = to_hex(&receiver_bytes); - let token_id_hex = to_hex(&token_id); - let msg_hex = to_hex(&cross_chain_message); - msg!( ... many msgs ... ); + #[cfg(feature = "verbose-cpi-logs")] + { + // Diagnostics: print CPI details and ABI message + let to_hex = |bytes: &[u8]| -> String { + let mut s = String::with_capacity(bytes.len() * 2); + for b in bytes { + use core::fmt::Write as _; + let _ = write!(&mut s, "{:02x}", b); + } + s + }; + let receiver_hex = to_hex(&receiver_bytes); + let token_id_hex = to_hex(&token_id); + let msg_hex = to_hex(&cross_chain_message); + let dest_hex = if is_stay_on_zetachain { + String::from("0x0000000000000000000000000000000000000000") + } else { to_hex(&final_destination_zrc20) }; + msg!("CPI invoke -> program_id (gateway): {}", ctx.accounts.config.gateway_program); + msg!(" Accounts[0] owner: {}, signer", owner_info.key()); + msg!(" Accounts[1] gateway_pda: {}", gateway_pda_info.key()); + msg!(" Args.receiver: 0x{}", receiver_hex); + msg!(" Args.message: destination={}, receiver=0x{}, tokenId=0x{}, uri='{}', sender=0x{}", dest_hex, to_hex(&receiver_addr), token_id_hex, uri, to_hex(&[0u8;20])); + msg!(" Encoded message len: {} (0x{})", cross_chain_message.len(), msg_hex); + }
150-160: Comment is misleading: helper does not create ATAsmint_nft_to_recipient only mints; it doesn’t create an ATA. Either create the ATA before mint or update the comment to reflect expectations.
Apply this diff:
- // Mint NFT to recipient (will create ATA if helper does so) + // Mint NFT to recipient (expects recipient ATA to pre-exist)
672-694: Avoid inconsistent gateway configuration: require new_gateway_pda when changing gateway programIf new_gateway_program is set without a matching new_gateway_pda in the same tx, the config can become unusable until a second update. Require both to be provided atomically.
Apply this diff:
// Validate and update gateway program if requested if let Some(gateway) = new_gateway_program { // Disallow zero pubkey; optionally check executability if the account is provided require!(gateway != Pubkey::default(), UniversalNftError::InvalidGatewayProgram); + // Ensure the new gateway PDA is provided in the same transaction to keep config consistent + require!(new_gateway_pda.is_some(), UniversalNftError::InvalidGatewayProgram); if let Some(acc) = ctx.remaining_accounts.iter().find(|a| a.key() == gateway) { // If the caller supplied the gateway account, ensure it's a program (executable) require!(acc.executable, UniversalNftError::InvalidGatewayProgram); } if gateway != config.gateway_program { config.gateway_program = gateway; } }contracts/nft/scripts/solana_devnet_nft.sh (1)
75-77: Consider graceful degradation for missing ts-node.Instead of exiting immediately, consider providing installation instructions or attempting to install ts-node automatically.
# Ensure ts-node is available if ! npx --yes ts-node --version >/dev/null 2>&1; then - echo -e "${RED}ts-node is required (dev dependency). Run: npm i -D ts-node typescript${NC}"; exit 1; + echo -e "${YELLOW}ts-node not found. Installing...${NC}" + npm install -D ts-node typescript || { + echo -e "${RED}Failed to install ts-node. Please run: npm i -D ts-node typescript${NC}" + exit 1 + } ficontracts/nft/contracts/example/SolanaUniversalNFT.ts (12)
15-17: Make RPC endpoint configurable via environment variablesDefaulting to devnet is fine for examples, but a CLI benefits from env overrides to switch clusters without code changes.
Apply this diff to allow overrides via SOLANA_RPC_URL or ANCHOR_PROVIDER_URL:
-const SOLANA_DEVNET_RPC = "https://api.devnet.solana.com"; +const SOLANA_DEVNET_RPC = process.env.SOLANA_RPC_URL + || process.env.ANCHOR_PROVIDER_URL + || "https://api.devnet.solana.com";
41-48: Support custom wallet path via env and improve error messageHardcoding ~/.config/solana/id.json limits CI/dev flows. Allow an override and guide the user when missing.
- private loadWallet(): Keypair { - const walletPath = path.join(homedir(), ".config", "solana", "id.json"); - if (!fs.existsSync(walletPath)) { - throw new Error("Wallet not found at ~/.config/solana/id.json"); - } + private loadWallet(): Keypair { + const walletPath = process.env.SOLANA_WALLET + || path.join(homedir(), ".config", "solana", "id.json"); + if (!fs.existsSync(walletPath)) { + throw new Error(`Wallet not found at ${walletPath}. Set SOLANA_WALLET to a keypair JSON path or create one via: solana-keygen new -o ${walletPath}`); + } const walletData = JSON.parse(fs.readFileSync(walletPath, "utf8")); return Keypair.fromSecretKey(new Uint8Array(walletData)); }
50-57: Initialize Program with explicit programId from IDL and assert address consistencyAnchoring the programId to the IDL’s address prevents subtle IDL/program mismatches at runtime.
- private loadProgram(): Program<UniversalNft> { - const provider = new anchor.AnchorProvider( - this.connection, - new anchor.Wallet(this.wallet), - { commitment: "confirmed" } - ); - return new Program(idl as UniversalNft, provider) as Program<UniversalNft>; - } + private loadProgram(): Program<UniversalNft> { + const provider = new anchor.AnchorProvider( + this.connection, + new anchor.Wallet(this.wallet), + { commitment: "confirmed" } + ); + const idlAddr = (idl as any)?.address; + if (!idlAddr) { + throw new Error("IDL is missing 'address'; ensure you built the IDL for the deployed program."); + } + const programId = new PublicKey(idlAddr); + const program = new Program(idl as anchor.Idl, programId, provider) as Program<UniversalNft>; + if (!program.programId.equals(programId)) { + throw new Error(`Program ID mismatch: Program(${program.programId.toBase58()}) vs IDL(${programId.toBase58()})`); + } + return program; + }
138-146: Add client-side caps for name/symbol/uri to fail-fast before on-chain errorsThe on-chain program enforces input caps. Validate early to save users from paying for failing transactions.
async mint(uri: string, name = "Universal NFT", symbol = "UNFT"): Promise<void> { + // Basic caps to mirror typical Metaplex/program expectations + const nameBytes = Buffer.byteLength(name, "utf8"); + const symbolBytes = Buffer.byteLength(symbol, "utf8"); + const uriBytes = Buffer.byteLength(uri, "utf8"); + if (nameBytes === 0 || nameBytes > 32) throw new Error(`name must be 1..32 bytes (got ${nameBytes})`); + if (symbolBytes === 0 || symbolBytes > 10) throw new Error(`symbol must be 1..10 bytes (got ${symbolBytes})`); + if (uriBytes === 0 || uriBytes > 200) throw new Error(`uri must be 1..200 bytes (got ${uriBytes})`);If your on-chain constants differ, adjust the limits above accordingly.
164-167: Consider setting a compute unit price for more reliable inclusionYou already boost the CU limit. Setting a modest CU price helps avoid CU exhaustion/priority starvation under load.
const computeIxs = [ ComputeBudgetProgram.setComputeUnitLimit({ units: 1_000_000 }), + // 1,000 micro-lamports per CU (~0.000001 SOL per 1k CU). Tune as needed. + ComputeBudgetProgram.setComputeUnitPrice({ microLamports: 1_000 }), ];
246-255: Fail-fast if NFT is already off-SolanaUsers get a clearer error before burning fails on-chain.
const origin = await this.program.account.nftOrigin.fetch(nftOriginPda); const mint = origin.originalMint as PublicKey; const metadataPda = origin.originalMetadata as PublicKey; + if (!(origin as any).isOnSolana) { + throw new Error("This NFT is currently off-Solana (bridged). Bring it back before initiating another transfer."); + }
256-263: Escalate IDL/program address mismatches to a hard errorYou log potential mismatches but still proceed. Abort early to avoid confusing Buffer offset/arg-mismatch errors later.
const idlAddr = (idl as any)?.address; if (idlAddr) { const idlInstr = (idl as any)?.instructions?.find((i: any) => i.name === 'transfer_to_zetachain' || i.name === 'transferToZetachain'); console.log('IDL program address:', idlAddr); console.log('Loaded program address:', this.program.programId.toBase58()); if (idlInstr) console.log('IDL transferToZetachain args:', idlInstr.args?.map((a: any) => `${a.name}:${JSON.stringify(a.type)}`)); + if (idlAddr !== this.program.programId.toBase58()) { + throw new Error(`IDL/program mismatch. Rebuild IDL (anchor build) and/or point the CLI to the correct deployed program.`); + } }
233-237: Broaden finalRecipient validation: accept 20-byte EVM hex OR ASCII ≤ 128 bytesOn-chain only caps the length (≤ 128). Restricting to hex EVM addresses limits flexibility. Accept either a 20-byte 0xhex or a non-empty ASCII string ≤ 128 bytes. Also update the usage message and pass the normalized value to the method.
Apply these diffs:
- Validation and normalization
- if (!finalRecipient) throw new Error("finalRecipient must be provided as a 20-byte EVM address (hex)"); - const recRaw = finalRecipient.trim(); - const recBuf = Buffer.from(recRaw.replace(/^0x/, ''), 'hex'); - if (recBuf.length !== 20) throw new Error(`finalRecipient must be a 20-byte hex EVM address (got ${recBuf.length} bytes)`); + if (!finalRecipient) throw new Error("finalRecipient must be provided"); + const recRaw = finalRecipient.trim(); + let finalRecipientNormalized = recRaw; + const hexMatch = recRaw.replace(/^0x/, ""); + if (/^[0-9a-fA-F]{40}$/.test(hexMatch)) { + // Normalize to 0x-prefixed lowercased hex + finalRecipientNormalized = "0x" + hexMatch.toLowerCase(); + } else { + const ascii = Buffer.from(recRaw, "utf8"); + if (ascii.length === 0 || ascii.length > 128) { + throw new Error(`finalRecipient must be a 20-byte hex address or an ASCII string ≤ 128 bytes (got ${ascii.length} bytes)`); + } + }
- Use the normalized value in the RPC call
- finalRecipient, + finalRecipientNormalized,
- Update CLI help text
- console.log("Commands:\n initialize <gatewayProgramPubkey> <gatewayPdaPubkey>\n update-config <newGatewayProgramPubkey|- or empty> <newGatewayPdaPubkey|- or empty> [newAuthorityPubkey|-] [pause true|false|-]\n mint <uri> [name] [symbol]\n transfer <tokenIdHex32> <zcUniversalContractHex20> <destZrc20Hex20|'0'> <finalRecipient> [depositSol default 0.02]\n status\n origin <tokenIdHex32>\n balance"); + console.log("Commands:\n initialize <gatewayProgramPubkey> <gatewayPdaPubkey>\n update-config <newGatewayProgramPubkey|- or empty> <newGatewayPdaPubkey|- or empty> [newAuthorityPubkey|-] [pause true|false|-]\n mint <uri> [name] [symbol]\n transfer <tokenIdHex32> <zcUniversalContractHex20> <destZrc20Hex20|'0'> <finalRecipient(20-byte 0xhex OR ASCII ≤128)> [depositSol default 0.02]\n status\n origin <tokenIdHex32>\n balance");Also applies to: 275-281, 407-407
294-297: Optional: add compute unit price alongside limit for the transferSame rationale as in mint(): give TX some priority headroom when burning + CPI to Gateway under load.
.preInstructions([ // Give headroom for burn + gateway CPI ComputeBudgetProgram.setComputeUnitLimit({ units: 600_000 }), + ComputeBudgetProgram.setComputeUnitPrice({ microLamports: 1_000 }), ])
347-349: Validate tokenId length in origin() like you do in transfer()Consistent validation improves UX.
async origin(tokenIdHex: string): Promise<void> { - const tokenId = Buffer.from(tokenIdHex.replace(/^0x/, ''), 'hex'); + const tokenId = Buffer.from(tokenIdHex.replace(/^0x/, ''), 'hex'); + if (tokenId.length !== 32) throw new Error(`tokenId must be 32 bytes hex (got ${tokenId.length})`);
360-365: Ensure non-zero exit on early constructor failures and avoid double-loggingErrors thrown before the try/catch (e.g., missing wallet) currently won’t set a non-zero exit code. Also, main().catch only logs. Move CLI construction into the try and set exitCode in the top-level catch.
async function main() { const [cmd, ...args] = process.argv.slice(2); - const cli = new UniversalNftCLI(); try { + const cli = new UniversalNftCLI();-if (require.main === module) { - main().catch(console.error); -} +if (require.main === module) { + main().catch((e) => { + console.error(e); + process.exitCode = 1; + }); +}Also applies to: 416-418
197-201: Minor: remove unused helpers if no longer neededtoU64LE and sha256 are currently unused. If they’re not slated for imminent use, consider removing them to reduce noise.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
contracts/nft/contracts/solana/Cargo.lockis excluded by!**/*.lockcontracts/nft/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
contracts/nft/contracts/example/SolanaUniversalNFT.ts(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs(1 hunks)contracts/nft/scripts/solana_devnet_nft.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: alperimo
PR: zeta-chain/standard-contracts#93
File: contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs:7-0
Timestamp: 2025-08-20T06:48:48.341Z
Learning: In Anchor Solana programs, the #[account] macro automatically implements AnchorSerialize, AnchorDeserialize, AccountSerialize, AccountDeserialize, Discriminator, Owner, and Clone traits for structs. Explicit derives for serialization are not needed and would be redundant for account data structures marked with #[account].
Learnt from: alperimo
PR: zeta-chain/standard-contracts#93
File: contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs:178-194
Timestamp: 2025-08-22T12:08:58.842Z
Learning: In the Solana universal-nft program, NFT_ORIGIN_SPACE constant already includes the 8-byte Anchor discriminator in its calculation (8 + 32 + 32 + 32 + 4 + MAX_URI_LENGTH + 8 + 8 + 1 + 1), and NftOrigin::LEN correctly uses this value for account space allocation.
Learnt from: alperimo
PR: zeta-chain/standard-contracts#93
File: contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs:7-0
Timestamp: 2025-08-20T06:48:48.341Z
Learning: In Anchor Solana programs, the #[account] macro automatically implements AnchorSerialize and AnchorDeserialize traits for structs, so explicit derives are not needed for account data structures.
📚 Learning: 2025-08-22T12:08:58.842Z
Learnt from: alperimo
PR: zeta-chain/standard-contracts#93
File: contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs:178-194
Timestamp: 2025-08-22T12:08:58.842Z
Learning: In the Solana universal-nft program, NFT_ORIGIN_SPACE constant already includes the 8-byte Anchor discriminator in its calculation (8 + 32 + 32 + 32 + 4 + MAX_URI_LENGTH + 8 + 8 + 1 + 1), and NftOrigin::LEN correctly uses this value for account space allocation.
Applied to files:
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs
🧬 Code graph analysis (2)
contracts/nft/scripts/solana_devnet_nft.sh (2)
contracts/nft/contracts/example/SolanaUniversalNFT.ts (2)
initialize(94-108)mint(138-209)contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (1)
initialize(28-66)
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (5)
contracts/nft/contracts/solana/programs/universal-nft/src/util/metaplex_helpers.rs (2)
create_metadata_account(15-81)create_master_edition_account(84-144)contracts/nft/contracts/solana/programs/universal-nft/src/util/cross_chain_helpers.rs (2)
decode_nft_data_with_recipient(44-78)hex_string_to_address(145-164)contracts/nft/contracts/solana/programs/universal-nft/src/util/mint_helpers.rs (1)
mint_nft_to_recipient(9-39)contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs (1)
new(40-58)contracts/nft/contracts/solana/programs/universal-nft/src/util/gateway_helpers.rs (2)
encode_evm_nft_message(43-89)encode_gateway_deposit_and_call_ix_data(23-35)
🔇 Additional comments (10)
contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs (3)
190-225: NftOrigin allocation/serialization looks correct (accounts for discriminator)You allocate using NftOrigin::SPACE and then serialize discriminator+data into the buffer. Per our prior learning, SPACE/LEN already includes the 8-byte Anchor discriminator. Good defensive zeroing before writes too.
129-144: Metadata creation ordering and idempotency checks look solidCreating metadata before mint, checking data_is_empty, and later gating master edition by supply==1 matches Metaplex invariants. Passing token_program to master edition was fixed appropriately.
68-98: Reservation logic and deterministic token_id derivation look goodCorrect use of slot and a monotonically-increasing reserved_id with checked_add; token_id derived as sha256(mint || slot_LE || reserved_id_LE) matches the documented formula.
contracts/nft/scripts/solana_devnet_nft.sh (5)
1-4: LGTM! Proper bash setup with error handling.The script correctly uses
set -euo pipefailfor robust error handling and includes a clear purpose statement.
13-25: Robust environment variable loading.The non-overriding .env loading mechanism is well-implemented, preventing environment variables from being accidentally overwritten while still providing sensible defaults.
88-92: Robust token ID extraction with proper error handling.The awk-based parsing correctly handles the "Token ID: " prefix and includes proper validation to ensure the script fails fast if the minting step doesn't produce the expected output.
96-106: Excellent error handling for transfer operation.The script properly captures both stdout/stderr and exit codes without immediately failing, allowing for comprehensive error reporting while still maintaining proper exit behavior.
29-35: Action Required: Manually Confirm Devnet Gateway AddressesI wasn’t able to automatically retrieve the current Solana gateway program and PDA for ZetaChain Devnet. Please check the official ZetaChain Devnet docs or CLI output to confirm that these defaults are still correct:
GATEWAY_PROGRAM(currentlyZETAjseVjuFsxdRxo6MmTCvqFwb3ZHUx56Co3vCmGis)GATEWAY_PDA(currently2f9SLuUNb7TNeM6gzBwT4ZjbL5ZyKzzHg1Ce9yiquEjj)If they’ve changed, update the script accordingly to avoid misrouting NFT transactions.
contracts/nft/contracts/example/SolanaUniversalNFT.ts (2)
66-71: PDAs derivation looks correct and aligned with Metaplex and program seedsSeeds and program IDs for config, metadata, master edition, origin, and tickets match the conventions described in the PR.
Also applies to: 73-78, 80-85, 87-92
197-207: Robust error surfacing with logs and stack tracesGood defensive logging for Anchor errors, including fetching logs from RpcResponseError. This will be valuable during testnet debugging.
Also applies to: 301-316
|
@alperimo yes! "Outbound mined" means a cross-chain transaction was successful, this is definitely a good sign. I can see the NFTs: (I recommend using Blockscout). |
|
Before we can proceed to reviewing the code, we need the following:
|
…nd implement EVM ABI decoding
…transfer commands
…eck for amount consistency
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
contracts/nft/scripts/testnet_nft.sh (3)
32-39: Base/BNB deploys: mirror resiliency and allow gateway override; validate outputs.Use retry, --no-install, env-overridable gateway, and address validation for both chains.
echo "🚀 Deploying EVMUniversalNFT on Base Sepolia (nft:deploy)..." -CONNECTED_BASE=$(npx hardhat nft:deploy --name EVMUniversalNFT --network base_sepolia --gateway 0x0c487a766110c85d301d96e33579c5b317fa4995 --json | jq -r '.contractAddress') +CONNECTED_BASE=$( + retry 3 npx --no-install hardhat nft:deploy \ + --name EVMUniversalNFT \ + --network base_sepolia \ + --gateway "${GATEWAY_BASE:-0x0c487a766110c85d301d96e33579c5b317fa4995}" \ + --gasLimit "$GAS_LIMIT" \ + --json | jq -r '.contractAddress' +) +is_eth_address "${CONNECTED_BASE}" || fail "Invalid Base Sepolia address: ${CONNECTED_BASE}" echo "Base Sepolia Universal NFT: $CONNECTED_BASE" echo "🚀 Deploying EVMUniversalNFT on BNB testnet (nft:deploy)..." -CONNECTED_BNB=$(npx hardhat nft:deploy --name EVMUniversalNFT --network bsc_testnet --gateway 0x0c487a766110c85d301d96e33579c5b317fa4995 --json | jq -r '.contractAddress') +CONNECTED_BNB=$( + retry 3 npx --no-install hardhat nft:deploy \ + --name EVMUniversalNFT \ + --network bsc_testnet \ + --gateway "${GATEWAY_BNB:-0x0c487a766110c85d301d96e33579c5b317fa4995}" \ + --gasLimit "$GAS_LIMIT" \ + --json | jq -r '.contractAddress' +) +is_eth_address "${CONNECTED_BNB}" || fail "Invalid BNB testnet address: ${CONNECTED_BNB}" echo "BNB testnet Universal NFT: $CONNECTED_BNB"
47-54: Network wiring: add retry and --no-install; fail fast on errors.External calls are flaky; wrap with retry and avoid auto-install.
echo "🔗 Wiring connections (set-connected / set-universal)..." -npx hardhat nft:set-connected --network zeta_testnet --contract "$UNIVERSAL" --connected "$CONNECTED_BASE" --zrc20 "$ZRC20_BASE" --json -npx hardhat nft:set-connected --network zeta_testnet --contract "$UNIVERSAL" --connected "$CONNECTED_BNB" --zrc20 "$ZRC20_BNB" --json -npx hardhat nft:set-connected --network zeta_testnet --contract "$UNIVERSAL" --connected "$CONNECTED_SOLANA" --zrc20 "$ZRC20_SOL" --json +retry 3 npx --no-install hardhat nft:set-connected --network zeta_testnet --contract "$UNIVERSAL" --connected "$CONNECTED_BASE" --zrc20 "$ZRC20_BASE" --json +retry 3 npx --no-install hardhat nft:set-connected --network zeta_testnet --contract "$UNIVERSAL" --connected "$CONNECTED_BNB" --zrc20 "$ZRC20_BNB" --json +retry 3 npx --no-install hardhat nft:set-connected --network zeta_testnet --contract "$UNIVERSAL" --connected "$CONNECTED_SOLANA" --zrc20 "$ZRC20_SOL" --json -npx hardhat nft:set-universal --network base_sepolia --contract "$CONNECTED_BASE" --universal "$UNIVERSAL" --json -npx hardhat nft:set-universal --network bsc_testnet --contract "$CONNECTED_BNB" --universal "$UNIVERSAL" --json +retry 3 npx --no-install hardhat nft:set-universal --network base_sepolia --contract "$CONNECTED_BASE" --universal "$UNIVERSAL" --json +retry 3 npx --no-install hardhat nft:set-universal --network bsc_testnet --contract "$CONNECTED_BNB" --universal "$UNIVERSAL" --json
61-68: Minting: add retry/--no-install and validate token IDs (jq may return null).Prevents silent downstream failures.
-BASE_MINT_JSON=$(npx hardhat nft:mint --network base_sepolia --contract "$CONNECTED_BASE" --token-uri "$NFT_URI" --name EVMUniversalNFT --gas-limit "$GAS_LIMIT" --json) +BASE_MINT_JSON=$(retry 3 npx --no-install hardhat nft:mint --network base_sepolia --contract "$CONNECTED_BASE" --tokenUri "$NFT_URI" --name EVMUniversalNFT --gasLimit "$GAS_LIMIT" --json) BASE_TOKEN_ID=$(echo "$BASE_MINT_JSON" | jq -r '.tokenId') +[[ -n "${BASE_TOKEN_ID}" && "${BASE_TOKEN_ID}" != "null" ]] || fail "Failed to parse Base tokenId: ${BASE_MINT_JSON}" echo "Base minted tokenId: $BASE_TOKEN_ID" -BNB_MINT_JSON=$(npx hardhat nft:mint --network bsc_testnet --contract "$CONNECTED_BNB" --token-uri "$NFT_URI" --name EVMUniversalNFT --gas-limit "$GAS_LIMIT" --json) +BNB_MINT_JSON=$(retry 3 npx --no-install hardhat nft:mint --network bsc_testnet --contract "$CONNECTED_BNB" --tokenUri "$NFT_URI" --name EVMUniversalNFT --gasLimit "$GAS_LIMIT" --json) BNB_TOKEN_ID=$(echo "$BNB_MINT_JSON" | jq -r '.tokenId') +[[ -n "${BNB_TOKEN_ID}" && "${BNB_TOKEN_ID}" != "null" ]] || fail "Failed to parse BNB tokenId: ${BNB_MINT_JSON}" echo "BNB minted tokenId: $BNB_TOKEN_ID"
🧹 Nitpick comments (7)
contracts/nft/scripts/testnet_nft.sh (2)
42-46: Parameterize ZRC-20 addresses and validate.Accept env overrides and ensure address format.
-# ZRC-20 references -ZRC20_BASE=0x236b0DE675cC8F46AE186897fCCeFe3370C9eDeD -ZRC20_BNB=0xd97B1de3619ed2c6BEb3860147E30cA8A7dC9891 -ZRC20_SOL=0xADF73ebA3Ebaa7254E859549A44c74eF7cff7501 +# ZRC-20 references (overridable) +ZRC20_BASE=${ZRC20_BASE:-0x236b0DE675cC8F46AE186897fCCeFe3370C9eDeD} +ZRC20_BNB=${ZRC20_BNB:-0xd97B1de3619ed2c6BEb3860147E30cA8A7dC9891} +ZRC20_SOL=${ZRC20_SOL:-0xADF73ebA3Ebaa7254E859549A44c74eF7cff7501} +is_eth_address "${ZRC20_BASE}" || fail "Invalid ZRC20_BASE" +is_eth_address "${ZRC20_BNB}" || fail "Invalid ZRC20_BNB" +is_eth_address "${ZRC20_SOL}" || fail "Invalid ZRC20_SOL"
4-4: Avoid leaking secrets in logs; gate xtrace by DEBUG.Hardhat may read PKs from env; xtrace can echo them.
-set -x +[[ -n "${DEBUG:-}" ]] && set -xcontracts/nft/contracts/solana/programs/universal-nft/src/util/cross_chain_helpers.rs (4)
47-66: Unify address hex parsing via hex::decode (simpler, safer).Avoid manual per-byte parsing and reuse the same crate you already use for encoding. Also returns clearer errors on invalid hex.
pub fn hex_string_to_address(address_str: &str) -> Result<[u8; 20]> { let hex_str = if address_str.starts_with("0x") { &address_str[2..] } else { address_str }; if hex_str.len() != 40 { return Err(UniversalNftError::InvalidRecipientAddress.into()); } - let mut bytes = [0u8; 20]; - for i in 0..20 { - let byte_str = &hex_str[i*2..i*2+2]; - bytes[i] = u8::from_str_radix(byte_str, 16) - .map_err(|_| UniversalNftError::InvalidRecipientAddress)?; - } - - Ok(bytes) + let mut bytes = [0u8; 20]; + let decoded = hex::decode(hex_str) + .map_err(|_| UniversalNftError::InvalidRecipientAddress)?; + if decoded.len() != 20 { + return Err(UniversalNftError::InvalidRecipientAddress.into()); + } + bytes.copy_from_slice(&decoded); + Ok(bytes) }
84-144: Tighten ABI validations: check 32-byte alignment and minimum offset; remove magic numbers.Prevents odd offsets and improves readability. Consider also capping URI length to your MAX_URI_LENGTH to avoid large allocations.
pub fn decode_evm_abi_nft_message(data: &[u8]) -> Result<([u8; 32], [u8; 20], String, u64, [u8; 20])> { - // Need at least 5 words for head and one word for string length - if data.len() < 32 * 6 { + // Need at least 5 words for head and one word for string length + const WORD: usize = 32; + const HEAD_WORDS: usize = 5; + if data.len() < WORD * (HEAD_WORDS + 1) { return Err(UniversalNftError::InvalidDataFormat.into()); } // Helper to extract a 32-byte word at index - let word = |i: usize| -> &[u8] { &data[i * 32..(i + 1) * 32] }; + let word = |i: usize| -> &[u8] { &data[i * WORD..(i + 1) * WORD] }; @@ - let offset = u64::from_be_bytes(off_u64_bytes) as usize; - if offset + 32 > data.len() { // must have length word + let offset = u64::from_be_bytes(off_u64_bytes) as usize; + // dynamic tail must start at/after the head and be 32-byte aligned + if offset < WORD * HEAD_WORDS || (offset % WORD != 0) { + return Err(UniversalNftError::InvalidDataFormat.into()); + } + if offset + WORD > data.len() { // must have length word return Err(UniversalNftError::InvalidDataFormat.into()); } @@ - let len_word = &data[offset..offset + 32]; + let len_word = &data[offset..offset + WORD]; @@ - let start = offset + 32; + let start = offset + WORD; if start + uri_len > data.len() { return Err(UniversalNftError::InvalidDataFormat.into()); }Follow-up (no diff): consider rejecting URIs longer than your on-chain limit (e.g., crate::constants::MAX_URI_LENGTH).
7-30: These length-prefixed string helpers are currently unused; either use them or silence warnings.If they’re kept for future use, add
#[allow(dead_code)](and consider a borrowed variant to avoid allocation)./// Extract a length-prefixed string from byte array /// Format: length(4 bytes, big-endian) + string_data -fn extract_length_prefixed_string(data: &[u8], cursor: usize) -> Result<(String, usize)> { +#[allow(dead_code)] +fn extract_length_prefixed_string(data: &[u8], cursor: usize) -> Result<(String, usize)> {
34-43: Guard against length overflow; consider returning Result from encoder.Casting
usizetou32can truncate for very large inputs. Make it fallible to avoid silent corruption (even if practically unlikely on Solana).-fn encode_length_prefixed_string(data: &mut Vec<u8>, string: &str) { +#[allow(dead_code)] +fn encode_length_prefixed_string(data: &mut Vec<u8>, string: &str) -> Result<()> { let string_bytes = string.as_bytes(); - let len = string_bytes.len() as u32; + if string_bytes.len() > u32::MAX as usize { + return Err(UniversalNftError::InvalidDataFormat.into()); + } + let len = string_bytes.len() as u32; @@ - data.extend_from_slice(string_bytes); + data.extend_from_slice(string_bytes); + Ok(()) }contracts/nft/contracts/solana/programs/universal-nft/src/context/on_call.rs (1)
25-27: Validate mint invariants at account validation time.Catch misconfigured mints early: decimals == 0 and mint_authority == pda.
Apply:
- #[account(mut)] - pub mint_account: Account<'info, Mint>, + #[account( + mut, + constraint = mint_account.decimals == 0, + constraint = mint_account.mint_authority == anchor_lang::solana_program::program_option::COption::Some(pda.key()) + )] + pub mint_account: Account<'info, Mint>,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
contracts/nft/contracts/example/SolanaUniversalNFT.ts(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/context/on_call.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/util/constants.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/util/cross_chain_helpers.rs(1 hunks)contracts/nft/scripts/testnet_nft.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- contracts/nft/contracts/example/SolanaUniversalNFT.ts
- contracts/nft/contracts/solana/programs/universal-nft/src/util/constants.rs
- contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: alperimo
PR: zeta-chain/standard-contracts#93
File: contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs:7-0
Timestamp: 2025-08-20T06:48:48.341Z
Learning: In Anchor Solana programs, the #[account] macro automatically implements AnchorSerialize, AnchorDeserialize, AccountSerialize, AccountDeserialize, Discriminator, Owner, and Clone traits for structs. Explicit derives for serialization are not needed and would be redundant for account data structures marked with #[account].
Learnt from: alperimo
PR: zeta-chain/standard-contracts#93
File: contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs:178-194
Timestamp: 2025-08-22T12:08:58.842Z
Learning: In the Solana universal-nft program, NFT_ORIGIN_SPACE constant already includes the 8-byte Anchor discriminator in its calculation (8 + 32 + 32 + 32 + 4 + MAX_URI_LENGTH + 8 + 8 + 1 + 1), and NftOrigin::LEN correctly uses this value for account space allocation.
Learnt from: alperimo
PR: zeta-chain/standard-contracts#93
File: contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs:7-0
Timestamp: 2025-08-20T06:48:48.341Z
Learning: In Anchor Solana programs, the #[account] macro automatically implements AnchorSerialize and AnchorDeserialize traits for structs, so explicit derives are not needed for account data structures.
📚 Learning: 2025-08-20T06:48:48.341Z
Learnt from: alperimo
PR: zeta-chain/standard-contracts#93
File: contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs:7-0
Timestamp: 2025-08-20T06:48:48.341Z
Learning: In Anchor Solana programs, the #[account] macro automatically implements AnchorSerialize, AnchorDeserialize, AccountSerialize, AccountDeserialize, Discriminator, Owner, and Clone traits for structs. Explicit derives for serialization are not needed and would be redundant for account data structures marked with #[account].
Applied to files:
contracts/nft/contracts/solana/programs/universal-nft/src/context/on_call.rs
🔇 Additional comments (7)
contracts/nft/scripts/testnet_nft.sh (2)
18-26: Compile step is solid.Good use of retry, actionable diagnostics, and IPv4-first hint.
28-31: Incorrect param naming concern
All flags in your deployment script use kebab-case and correctly correspond to the Hardhat task parameters (name,network,gateway,uniswap-router,json). There is no mix of camelCase flags here, so this check can be safely dismissed.Likely an incorrect or invalid review comment.
contracts/nft/contracts/solana/programs/universal-nft/src/util/cross_chain_helpers.rs (1)
68-71: Dependency Verification Complete:hexcrate is declared
- Confirmed usage of
hex::encodein
contracts/nft/contracts/solana/programs/universal-nft/src/util/cross_chain_helpers.rs(lines 69–71).- Confirmed declaration of
hex = "0.4"in
contracts/nft/contracts/solana/programs/universal-nft/Cargo.toml(line 23).Optional UX Improvement
If the resulting hex string is displayed to end users, consider returning an EIP-55-checksummed address instead of all-lowercase hex for improved readability and error detection. Otherwise, the current lowercase output is perfectly acceptable for internal usage.contracts/nft/contracts/solana/programs/universal-nft/src/context/on_call.rs (4)
9-13: Mark config PDA as mut if it funds lazy account creations.If on_call lazily creates metadata/master edition and pays from the config PDA, pda must be mutable to debit lamports.
Apply this diff if pda funds creations:
#[account( - seeds = [UNIVERSAL_NFT_CONFIG_SEED], - bump = pda.bump + mut, // needed if PDA pays fees for lazy init + seeds = [UNIVERSAL_NFT_CONFIG_SEED], + bump = pda.bump )]If you don’t spend from the PDA, ignore this.
29-41: Metaplex Metadata PDA constraints look correct.
43-56: Master Edition PDA constraints look correct.
58-65: Gateway PDA pinning and ownership check — good.This complements the runtime previous-instruction check.
| /// NFT origin account (PDA) derived as [NFT_ORIGIN_SEED, origin_chain.to_be_bytes(), token_id] | ||
| /// CHECK: Passed PDA must match derived address; we (re)initialize if empty | ||
| #[account(mut)] | ||
| pub nft_origin: UncheckedAccount<'info>, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Statically enforce nft_origin PDA derivation using #[instruction(...)] seeds.
This removes a runtime footgun and ensures the passed account matches the canonical PDA.
Apply:
-#[derive(Accounts)]
+#[derive(Accounts)]
+#[instruction(origin_chain: u64, token_id: [u8; 32])]
pub struct OnCall<'info> {
@@
- #[account(mut)]
- pub nft_origin: UncheckedAccount<'info>,
+ #[account(
+ mut,
+ seeds = [
+ crate::util::constants::NFT_ORIGIN_SEED,
+ &origin_chain.to_be_bytes(),
+ token_id.as_ref(),
+ ],
+ bump
+ )]
+ pub nft_origin: UncheckedAccount<'info>,And extend the import if needed:
-use crate::util::constants::{UNIVERSAL_NFT_CONFIG_SEED, TOKEN_METADATA_PROGRAM_ID};
+use crate::util::constants::{UNIVERSAL_NFT_CONFIG_SEED, NFT_ORIGIN_SEED, TOKEN_METADATA_PROGRAM_ID};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// NFT origin account (PDA) derived as [NFT_ORIGIN_SEED, origin_chain.to_be_bytes(), token_id] | |
| /// CHECK: Passed PDA must match derived address; we (re)initialize if empty | |
| #[account(mut)] | |
| pub nft_origin: UncheckedAccount<'info>, | |
| // Extend your imports to pull in the NFT_ORIGIN_SEED constant | |
| use crate::util::constants::{UNIVERSAL_NFT_CONFIG_SEED, NFT_ORIGIN_SEED, TOKEN_METADATA_PROGRAM_ID}; | |
| #[derive(Accounts)] | |
| // Declare the instruction arguments so we can reference them in the account constraints | |
| #[instruction(origin_chain: u64, token_id: [u8; 32])] | |
| pub struct OnCall<'info> { | |
| /// NFT origin account (PDA) derived as [NFT_ORIGIN_SEED, origin_chain.to_be_bytes(), token_id] | |
| /// CHECK: Passed PDA must match derived address; we (re)initialize if empty | |
| #[account( | |
| mut, | |
| seeds = [ | |
| crate::util::constants::NFT_ORIGIN_SEED, | |
| &origin_chain.to_be_bytes(), | |
| token_id.as_ref(), | |
| ], | |
| bump | |
| )] | |
| pub nft_origin: UncheckedAccount<'info>, | |
| // …other accounts… | |
| } |
🤖 Prompt for AI Agents
In contracts/nft/contracts/solana/programs/universal-nft/src/context/on_call.rs
around lines 15-18, the nft_origin PDA is only runtime-checked; add a
compile-time PDA constraint by requiring the handler to declare the PDA seed
inputs with an #[instruction(...)] and annotate the account with seeds and bump.
Specifically, add an #[instruction(origin_chain: u64, token_id: Vec<u8>,
nft_origin_bump: u8)] to the instruction handler signature, change the account
attribute to #[account(mut, seeds = [NFT_ORIGIN_SEED,
origin_chain.to_be_bytes().as_ref(), token_id.as_ref()], bump =
nft_origin_bump)] on nft_origin, and import any needed Anchor attributes/types
if not already present.
| /// Program ATA to receive minted token (authority = config, mint = mint) | ||
| /// We validate at runtime that it matches the derived ATA | ||
| #[account(mut)] | ||
| pub pda_ata: Account<'info, TokenAccount>, | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enforce ATA derivation for pda_ata to prevent spoofed token accounts.
Anchor can verify the ATA at account-validation time.
Apply this diff and add the Associated Token Program to the context:
- #[account(mut)]
- pub pda_ata: Account<'info, TokenAccount>,
+ #[account(
+ mut,
+ associated_token::mint = mint_account,
+ associated_token::authority = pda
+ )]
+ pub pda_ata: Account<'info, TokenAccount>,
@@
- /// Token program for NFT operations
- pub token_program: Program<'info, Token>,
+ /// Token program for NFT operations
+ pub token_program: Program<'info, Token>,
+
+ /// Associated Token program (for ATA checks/creation)
+ pub associated_token_program: Program<'info, anchor_spl::associated_token::AssociatedToken>,At the top, ensure the import exists:
-use anchor_spl::token::{Token, TokenAccount, Mint};
+use anchor_spl::token::{Token, TokenAccount, Mint};
+use anchor_spl::associated_token::AssociatedToken;🤖 Prompt for AI Agents
In contracts/nft/contracts/solana/programs/universal-nft/src/context/on_call.rs
around lines 20 to 24, the pda_ata field must be validated as the ATA derived
for (mint, config authority) and the Associated Token Program must be added to
the context and imports; update the account attribute to use seeds and
constraint that verifies the PDA ATA derivation (e.g. with
associated_token::get_associated_token_address equivalent) so the account is
checked at validation time, add a new pub associated_token_program:
Program<'info, AssociatedToken> to the struct, and add the AssociatedToken
import at the top of the file (use anchor_spl::associated_token::AssociatedToken
or equivalent).
| /// System program for account creation | ||
| pub system_program: Program<'info, System>, | ||
|
|
||
| /// Token program for NFT operations | ||
| pub token_program: Program<'info, Token>, | ||
|
|
||
| /// Rent sysvar | ||
| pub rent: Sysvar<'info, Rent>, | ||
| } No newline at end of file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add the Instructions sysvar; without it, on_call cannot authenticate Gateway CPI.
Right now anyone can directly invoke on_call and pass a valid gateway_pda account. You need the instructions sysvar to verify the previous instruction’s program_id equals pda.gateway_program. Add it to the context and use it in the handler.
Apply this diff:
@@
- /// System program for account creation
+ /// Sysvar: Instructions — required to verify CPI caller (Gateway)
+ /// CHECK: Must be the well-known sysvar address
+ #[account(address = anchor_lang::solana_program::sysvar::instructions::ID)]
+ pub ix_sysvar: UncheckedAccount<'info>,
+
+ /// System program for account creation
pub system_program: Program<'info, System>,
@@
- /// Token program for NFT operations
+ /// Token program for NFT operations
pub token_program: Program<'info, Token>,In the handler (outside this file), enforce:
use anchor_lang::solana_program::sysvar::instructions as ix_sysvar;
let ix_ai = &ctx.accounts.ix_sysvar;
let idx = ix_sysvar::load_current_index_checked(ix_ai)?;
let prev = ix_sysvar::load_instruction_at_checked((idx.saturating_sub(1)) as usize, ix_ai)?;
require!(prev.program_id == ctx.accounts.pda.gateway_program, UniversalNftError::InvalidGatewayProgram);
require!(prev.accounts.iter().any(|m| m.pubkey == ctx.accounts.gateway_pda.key()), UniversalNftError::InvalidGatewayProgram);Run to confirm the handler actually performs this check:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -nP -C3 --type=rust 'on_call\s*\(' contracts/nft/contracts/solana/programs/universal-nft
rg -nP -C3 --type=rust '(sysvar::instructions|load_current_index|load_instruction_at)' contracts/nft/contracts/solana/programs/universal-nftLength of output: 2605
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print context around the on_call handler to verify gateway CPI checks
sed -n '450,550p' contracts/nft/contracts/solana/programs/universal-nft/src/lib.rsLength of output: 4565
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and display the OnCall accounts struct to check for the instructions sysvar field
rg -nP -C5 'pub struct OnCall' --type=rust contracts/nft/contracts/solana/programs/universal-nft/src/context/on_call.rs || true
# Print the first 200 lines of the file for context
head -n 200 contracts/nft/contracts/solana/programs/universal-nft/src/context/on_call.rsLength of output: 3211
Enforce the Instructions Sysvar in the OnCall Context
To guarantee at‐compile time that the instruction sysvar is supplied—and prevent any invocation of on_call without proper Gateway CPI authentication—add the sysvar to your OnCall accounts struct and update the handler to use it directly.
• In contracts/nft/contracts/solana/programs/universal-nft/src/context/on_call.rs, insert before system_program:
@@ pub struct OnCall<'info> {
/// Gateway PDA, used for minimal caller verification
#[account(
constraint = gateway_pda.key() == pda.gateway_pda @ crate::error::UniversalNftError::InvalidGatewayProgram,
constraint = *gateway_pda.owner == pda.gateway_program @ crate::error::UniversalNftError::InvalidGatewayProgram
)]
pub gateway_pda: UncheckedAccount<'info>,
+ /// Sysvar: Instructions — required to verify Gateway CPI
+ /// CHECK: must be the well-known sysvar instructions address
+ #[account(address = anchor_lang::solana_program::sysvar::instructions::ID)]
+ pub ix_sysvar: UncheckedAccount<'info>,
/// System program for account creation
pub system_program: Program<'info, System>,• In the on_call handler (e.g. in src/lib.rs), replace the remaining_accounts lookup with:
let ix_account_info = ctx.accounts.ix_sysvar.to_account_info();
let current_ix = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative(
-1,
&ix_account_info,
).map_err(|_| UniversalNftError::InvalidCaller)?;
require!(
current_ix.program_id == ctx.accounts.pda.gateway_program,
UniversalNftError::InvalidCaller
);This change ensures Anchor enforces the presence and correctness of the instructions sysvar at deployment time—eliminating reliance on remaining_accounts scans and avoiding silent misconfiguration.
🤖 Prompt for AI Agents
In contracts/nft/contracts/solana/programs/universal-nft/src/context/on_call.rs
around lines 66–74, add an Instructions sysvar field to the OnCall accounts
struct (insert a field like ix_sysvar: Sysvar<'info, Instructions'> immediately
before system_program) so Anchor enforces the presence of the instructions
sysvar; then in the on_call handler (e.g. src/lib.rs) remove the
remaining_accounts lookup and instead call
ctx.accounts.ix_sysvar.to_account_info(), pass that to
anchor_lang::solana_program::sysvar::instructions::get_instruction_relative(-1,
&ix_account_info) mapping errors to UniversalNftError::InvalidCaller, and
require that the returned instruction's program_id equals
ctx.accounts.pda.gateway_program; also add the necessary import for
get_instruction_relative and ensure UniversalNftError::InvalidCaller is used for
failures.
| echo "🚀 Deploying Universal NFT on ZetaChain testnet (nft:deploy)..." | ||
| UNIVERSAL=$(npx hardhat nft:deploy --name ZetaChainUniversalNFT --network zeta_testnet --gateway 0x6c533f7fe93fae114d0954697069df33c9b74fd7 --uniswap-router 0x2ca7d64A7EFE2D62A725E2B35Cf7230D6677FfEe --json | jq -r '.contractAddress') | ||
| echo "ZetaChain Universal NFT: $UNIVERSAL" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix flag name and harden ZetaChain deploy: add retry/--no-install and validate address.
Flag likely should be --uniswapRouter (camelCase), not --uniswap-router. Add retry, avoid auto-install, and fail if jq returns null/invalid.
echo "🚀 Deploying Universal NFT on ZetaChain testnet (nft:deploy)..."
-UNIVERSAL=$(npx hardhat nft:deploy --name ZetaChainUniversalNFT --network zeta_testnet --gateway 0x6c533f7fe93fae114d0954697069df33c9b74fd7 --uniswap-router 0x2ca7d64A7EFE2D62A725E2B35Cf7230D6677FfEe --json | jq -r '.contractAddress')
+UNIVERSAL=$(
+ retry 3 npx --no-install hardhat nft:deploy \
+ --name ZetaChainUniversalNFT \
+ --network zeta_testnet \
+ --gateway "${GATEWAY_ZETA:-0x6c533f7fe93fae114d0954697069df33c9b74fd7}" \
+ --uniswapRouter "${UNISWAP_ROUTER_ZETA:-0x2ca7d64A7EFE2D62A725E2B35Cf7230D6677FfEe}" \
+ --gasLimit "$GAS_LIMIT" \
+ --json | jq -r '.contractAddress'
+)
+if [[ -z "${UNIVERSAL:-}" || "${UNIVERSAL}" == "null" || ! "${UNIVERSAL}" =~ ^0x[0-9a-fA-F]{40}$ ]]; then
+ echo "Failed to obtain Universal NFT address on zeta_testnet. Got: '${UNIVERSAL}'" >&2
+ exit 1
+fi
echo "ZetaChain Universal NFT: $UNIVERSAL"Additional helper functions to add once (place near the retry helper):
# helpers
is_eth_address() { [[ "$1" =~ ^0x[0-9a-fA-F]{40}$ ]]; }
fail() { echo "error: $*" >&2; exit 1; }🤖 Prompt for AI Agents
In contracts/nft/scripts/testnet_nft.sh around lines 28-31, the deploy command
uses the wrong flag name and is brittle; update the hardhat invocation to use
the camelCase flag (--uniswapRouter) and add --no-install to avoid
auto-installing, wrap the npx hardhat call with the existing retry helper so it
retries on transient failures, capture the JSON output then validate the address
returned by jq with an is_eth_address helper and call fail if it's null/invalid;
also add the two helper functions once near the retry helper: is_eth_address
(regex check for 0x + 40 hex chars) and fail (prints to stderr and exits
non-zero).
| CONNECTED_SOLANA="0x48727a61636b6973564d4c436a774a6e7531364b766b6b4843735038726964735a36755176667a697952736e" # TODO: SET HEX OF SOLANA_PROGRAM_ID (e.g: HrzackisVMLCjwJnu16KvkkHCsP8ridsZ6uQvfziyRsn) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate Solana program-id hex and allow override.
Guard against placeholder/typos; fail fast if malformed.
-CONNECTED_SOLANA="0x48727a61636b6973564d4c436a774a6e7531364b766b6b4843735038726964735a36755176667a697952736e" # TODO: SET HEX OF SOLANA_PROGRAM_ID (e.g: HrzackisVMLCjwJnu16KvkkHCsP8ridsZ6uQvfziyRsn)
+CONNECTED_SOLANA="${CONNECTED_SOLANA:-0x48727a61636b6973564d4c436a774a6e7531364b766b6b4843735038726964735a36755176667a697952736e}" # hex(Solana program id)
+[[ "${CONNECTED_SOLANA}" =~ ^0x[0-9a-fA-F]+$ ]] || fail "CONNECTED_SOLANA must be 0x-hex"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In contracts/nft/scripts/testnet_nft.sh around lines 40-41, the hardcoded
CONNECTED_SOLANA hex may be a placeholder or malformed; add validation and an
override: read an env var (e.g. CONNECTED_SOLANA_HEX) or CLI arg first, then
validate the chosen value starts with "0x" followed by exactly 64 hex characters
(0-9a-f/A-F); if validation fails, print a clear error and exit non-zero
(fail-fast). Ensure the script uses the validated value thereafter and include a
fallback message instructing how to set the env var or pass the argument.
| BASE_XFER_JSON=$(npx hardhat nft:transfer \ | ||
| --network base_sepolia \ | ||
| --contract "$CONNECTED_BASE" \ | ||
| --token-id "$BASE_TOKEN_ID" \ | ||
| --destination "$ZRC20_SOL" \ | ||
| --gas-amount 0.005 \ | ||
| --json) | ||
| echo "$BASE_XFER_JSON" | jq -r '. | "🚀 Successfully transferred NFT to the contract.\n📜 Contract address: \(.contractAddress)\n🖼 NFT Contract address: \(.contractAddress)\n🆔 Token ID: \(.tokenId)\n🔗 Transaction hash: \(.transferTransactionHash)\n⛽ Gas used: \(.gasUsed)"' | ||
| BASE_TX_HASH=$(echo "$BASE_XFER_JSON" | jq -r '.transferTransactionHash') | ||
| echo "🔎 Inbound CCTX: https://zetachain-athens.blockpi.network/lcd/v1/public/zeta-chain/crosschain/inboundHashToCctxData/$BASE_TX_HASH" | ||
| echo "🔎 Base Sepolia tx: https://sepolia.basescan.org/tx/$BASE_TX_HASH" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Base→Solana transfer: add retry and validate tx hash; reduce noisy output duplication.
Treat network flakiness and ensure non-null hash before printing links.
-BASE_XFER_JSON=$(npx hardhat nft:transfer \
+BASE_XFER_JSON=$(retry 3 npx --no-install hardhat nft:transfer \
--network base_sepolia \
--contract "$CONNECTED_BASE" \
--token-id "$BASE_TOKEN_ID" \
--destination "$ZRC20_SOL" \
--gas-amount 0.005 \
--json)
-echo "$BASE_XFER_JSON" | jq -r '. | "🚀 Successfully transferred NFT to the contract.\n📜 Contract address: \(.contractAddress)\n🖼 NFT Contract address: \(.contractAddress)\n🆔 Token ID: \(.tokenId)\n🔗 Transaction hash: \(.transferTransactionHash)\n⛽ Gas used: \(.gasUsed)"'
+echo "$BASE_XFER_JSON" | jq -r '. | "🚀 Transfer initiated.\n📜 Contract: \(.contractAddress)\n🆔 Token ID: \(.tokenId)\n🔗 Tx: \(.transferTransactionHash)\n⛽ Gas used: \(.gasUsed)"'
BASE_TX_HASH=$(echo "$BASE_XFER_JSON" | jq -r '.transferTransactionHash')
+[[ -n "${BASE_TX_HASH}" && "${BASE_TX_HASH}" != "null" ]] || fail "Missing Base transfer tx hash: ${BASE_XFER_JSON}"
echo "🔎 Inbound CCTX: https://zetachain-athens.blockpi.network/lcd/v1/public/zeta-chain/crosschain/inboundHashToCctxData/$BASE_TX_HASH"
echo "🔎 Base Sepolia tx: https://sepolia.basescan.org/tx/$BASE_TX_HASH"Committable suggestion skipped: line range outside the PR's diff.
| BNB_XFER_JSON=$(npx hardhat nft:transfer \ | ||
| --network bsc_testnet \ | ||
| --contract "$CONNECTED_BNB" \ | ||
| --token-id "$BNB_TOKEN_ID" \ | ||
| --destination "$ZRC20_SOL" \ | ||
| --gas-amount 0.005 \ | ||
| --json) | ||
| echo "$BNB_XFER_JSON" | jq -r '. | "🚀 Successfully transferred NFT to the contract.\n📜 Contract address: \(.contractAddress)\n🖼 NFT Contract address: \(.contractAddress)\n🆔 Token ID: \(.tokenId)\n🔗 Transaction hash: \(.transferTransactionHash)\n⛽ Gas used: \(.gasUsed)"' | ||
| BNB_TX_HASH=$(echo "$BNB_XFER_JSON" | jq -r '.transferTransactionHash') | ||
| echo "🔎 Inbound CCTX: https://zetachain-athens.blockpi.network/lcd/v1/public/zeta-chain/crosschain/inboundHashToCctxData/$BNB_TX_HASH" | ||
| echo "🔎 BNB Testnet tx: https://testnet.bscscan.com/tx/$BNB_TX_HASH" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
BNB→Solana transfer: same retry/hash validation as above.
Mirror robustness.
-BNB_XFER_JSON=$(npx hardhat nft:transfer \
+BNB_XFER_JSON=$(retry 3 npx --no-install hardhat nft:transfer \
--network bsc_testnet \
--contract "$CONNECTED_BNB" \
--token-id "$BNB_TOKEN_ID" \
--destination "$ZRC20_SOL" \
--gas-amount 0.005 \
--json)
echo "$BNB_XFER_JSON" | jq -r '. | "🚀 Successfully transferred NFT to the contract.\n📜 Contract address: \(.contractAddress)\n🖼 NFT Contract address: \(.contractAddress)\n🆔 Token ID: \(.tokenId)\n🔗 Transaction hash: \(.transferTransactionHash)\n⛽ Gas used: \(.gasUsed)"'
BNB_TX_HASH=$(echo "$BNB_XFER_JSON" | jq -r '.transferTransactionHash')
+[[ -n "${BNB_TX_HASH}" && "${BNB_TX_HASH}" != "null" ]] || fail "Missing BNB transfer tx hash: ${BNB_XFER_JSON}"
echo "🔎 Inbound CCTX: https://zetachain-athens.blockpi.network/lcd/v1/public/zeta-chain/crosschain/inboundHashToCctxData/$BNB_TX_HASH"
echo "🔎 BNB Testnet tx: https://testnet.bscscan.com/tx/$BNB_TX_HASH"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In contracts/nft/scripts/testnet_nft.sh around lines 85 to 96, this BNB→Solana
transfer block is missing the same retry/poll and hash-validation logic used
elsewhere; add the same robust retry loop that polls the inbound CCTX endpoint
until the cross-chain inbound record appears, validate the returned
transferTransactionHash (and relevant fields like contractAddress/tokenId)
matches $BNB_TX_HASH (or fail after a timeout/retry limit), surface errors and
non-zero exit on failure, and log progress/status exactly like the other
transfer paths so transfers are retried and verified consistently.
@fadeev as you can see, my on_call must receive nft_origin, metadata, and master_edition as accounts. I need them because i must verify/initialize the origin record and create/update Metaplex metadata and master edition PDAs during the mint/assign step. The question is: How to include them directly in my on_call context. Should i change the relayer so that it includes them as remaining accounts when calling the gateway execute or should i move the nft creation to the another/second instruction like claim/complete_mint where a client passes those accounts explicitly? |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contracts/nft/contracts/solana/programs/universal-nft/src/util/constants.rs (1)
102-108: Gate Gateway Program ID by Cargo feature flags.Prevents accidental deployment with devnet ID on mainnet.
-/// ZetaChain Gateway Program ID (placeholder - replace with actual deployed gateway) -/// Note: This should be updated with the real ZetaChain gateway program ID for production -/// For devnet, use: "94U5AHQMKkV5txNJ17QPXWoh474PheGou6cNP2FEuL1d" -/// For mainnet, use: "ZETAjseVjuFsxdRxo6MmTCvqFwb3ZHUx56Co3vCmGis" - -pub const ZETACHAIN_GATEWAY_PROGRAM_ID: Pubkey = pubkey!("94U5AHQMKkV5txNJ17QPXWoh474PheGou6cNP2FEuL1d"); +/// ZetaChain Gateway Program ID (selected by Cargo feature) +#[cfg(feature = "mainnet")] +pub const ZETACHAIN_GATEWAY_PROGRAM_ID: Pubkey = pubkey!("ZETAjseVjuFsxdRxo6MmTCvqFwb3ZHUx56Co3vCmGis"); +#[cfg(not(feature = "mainnet"))] +pub const ZETACHAIN_GATEWAY_PROGRAM_ID: Pubkey = pubkey!("94U5AHQMKkV5txNJ17QPXWoh474PheGou6cNP2FEuL1d");
🧹 Nitpick comments (8)
contracts/nft/contracts/example/SolanaUniversalNFT.ts (5)
15-16: Make RPC endpoint configurable via env, keep devnet as fallback.Improves portability and avoids editing code per environment.
-const SOLANA_DEVNET_RPC = "https://api.devnet.solana.com"; +const SOLANA_RPC = process.env.SOLANA_RPC_URL || "https://api.devnet.solana.com"; @@ - this.connection = new Connection(SOLANA_DEVNET_RPC, "confirmed"); + this.connection = new Connection(SOLANA_RPC, "confirmed");Also applies to: 35-39
18-28: Drop unused helpers or wire them in.
toU64LEandsha256are currently unused.
255-262: Gate verbose IDL diagnostics behind a DEBUG flag.Prevents noisy logs in normal runs.
- const idlAddr = (idl as any)?.address; - if (idlAddr) { - const idlInstr = (idl as any)?.instructions?.find((i: any) => i.name === 'transfer_to_zetachain' || i.name === 'transferToZetachain'); - console.log('IDL program address:', idlAddr); - console.log('Loaded program address:', this.program.programId.toBase58()); - if (idlInstr) console.log('IDL transferToZetachain args:', idlInstr.args?.map((a: any) => `${a.name}:${JSON.stringify(a.type)}`)); - } + if (process.env.DEBUG_CLI) { + const idlAddr = (idl as any)?.address; + if (idlAddr) { + const idlInstr = (idl as any)?.instructions?.find((i: any) => i.name === 'transfer_to_zetachain' || i.name === 'transferToZetachain'); + console.log('IDL program address:', idlAddr); + console.log('Loaded program address:', this.program.programId.toBase58()); + if (idlInstr) console.log('IDL transferToZetachain args:', idlInstr.args?.map((a: any) => `${a.name}:${JSON.stringify(a.type)}`)); + } + }
292-296: Add a small CU price to improve inclusion under load.Keeps CU limit plus priority fee; harmless on low congestion.
.preInstructions([ // Give headroom for burn + gateway CPI ComputeBudgetProgram.setComputeUnitLimit({ units: 600_000 }), + // Optional: priority fee (micro-lamports per CU) + ComputeBudgetProgram.setComputeUnitPrice({ microLamports: Number(process.env.CU_PRICE_MICROLAMPORTS ?? 0) }), ])
405-406: Clarify usage: finalRecipient is an EVM address.Avoids confusion about expected format.
- console.log("Commands:\n initialize <gatewayProgramPubkey> <gatewayPdaPubkey>\n update-config <newGatewayProgramPubkey|- or empty> <newGatewayPdaPubkey|- or empty> [newAuthorityPubkey|-] [pause true|false|-]\n mint <uri> [name] [symbol]\n transfer <tokenIdHex32> <zcUniversalContractHex20> <destZrc20Hex20|'0'> <finalRecipient> [depositSol default 0.02]\n status\n origin <tokenIdHex32>\n balance"); + console.log("Commands:\n initialize <gatewayProgramPubkey> <gatewayPdaPubkey>\n update-config <newGatewayProgramPubkey|- or empty> <newGatewayPdaPubkey|- or empty> [newAuthorityPubkey|-] [pause true|false|-]\n mint <uri> [name] [symbol]\n transfer <tokenIdHex32> <zcUniversalContractHex20> <destZrc20Hex20|'0'> <finalRecipientEvmHex20> [depositSol default 0.02]\n status\n origin <tokenIdHex32>\n balance");contracts/nft/contracts/solana/programs/universal-nft/src/context/transfer_to_zetachain.rs (3)
6-6: Remove unused import TOKEN_METADATA_PROGRAM_ID.Avoids warnings; not used in this context.
-use crate::util::constants::{*, TOKEN_METADATA_PROGRAM_ID}; +use crate::util::constants::*;
31-37: ATA constraint set is good; consider also gating delegated balance (optional).If delegations are possible, you may additionally reject delegated transfers.
Example (if using Anchor >=0.29 supports the field access in constraints):
#[account( mut, associated_token::mint = mint, associated_token::authority = owner, - constraint = token_account.amount == 1 @ crate::error::UniversalNftError::InvalidTokenAmount + constraint = token_account.amount == 1 @ crate::error::UniversalNftError::InvalidTokenAmount, + constraint = token_account.delegated_amount == 0 @ crate::error::UniversalNftError::InvalidTokenAccount )]
8-10: on_call account sourcing: favor remaining_accounts from relayer.Keep on-chain idempotent but avoid client second step by letting relayer supply
nft_origin,metadata,master_editionas remaining accounts (derive/verify seeds on-chain).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
contracts/nft/contracts/example/SolanaUniversalNFT.ts(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/context/transfer_to_zetachain.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/error/mod.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs(1 hunks)contracts/nft/contracts/solana/programs/universal-nft/src/util/constants.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- contracts/nft/contracts/solana/programs/universal-nft/src/error/mod.rs
- contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: alperimo
PR: zeta-chain/standard-contracts#93
File: contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs:7-0
Timestamp: 2025-08-20T06:48:48.341Z
Learning: In Anchor Solana programs, the #[account] macro automatically implements AnchorSerialize, AnchorDeserialize, AccountSerialize, AccountDeserialize, Discriminator, Owner, and Clone traits for structs. Explicit derives for serialization are not needed and would be redundant for account data structures marked with #[account].
Learnt from: alperimo
PR: zeta-chain/standard-contracts#93
File: contracts/nft/contracts/solana/programs/universal-nft/src/lib.rs:178-194
Timestamp: 2025-08-22T12:08:58.842Z
Learning: In the Solana universal-nft program, NFT_ORIGIN_SPACE constant already includes the 8-byte Anchor discriminator in its calculation (8 + 32 + 32 + 32 + 4 + MAX_URI_LENGTH + 8 + 8 + 1 + 1), and NftOrigin::LEN correctly uses this value for account space allocation.
Learnt from: alperimo
PR: zeta-chain/standard-contracts#93
File: contracts/nft/contracts/solana/programs/universal-nft/src/state/nft_origin.rs:7-0
Timestamp: 2025-08-20T06:48:48.341Z
Learning: In Anchor Solana programs, the #[account] macro automatically implements AnchorSerialize and AnchorDeserialize traits for structs, so explicit derives are not needed for account data structures.
🔇 Additional comments (1)
contracts/nft/contracts/example/SolanaUniversalNFT.ts (1)
59-65: Seed parity confirmed – The TypeScriptderiveConfigPdauses"connected", which exactly matches the RustUNIVERSAL_NFT_CONFIG_SEED: &[u8] = b"connected";constant, so no changes are needed.
| // Strict arg validations | ||
| const tokenId = Buffer.from(tokenIdHex.replace(/^0x/, ''), 'hex'); | ||
| if (tokenId.length !== 32) throw new Error(`tokenId must be 32 bytes hex (got ${tokenId.length})`); | ||
| const receiver = Buffer.from(zcUniversalContractHex.replace(/^0x/, ''), 'hex'); | ||
| if (receiver.length !== 20) throw new Error(`zetachain universal contract must be 20 bytes hex (got ${receiver.length})`); | ||
| // destinationZrc20: accept '0' (stay on ZetaChain) or 20-byte hex address | ||
| const destRaw = destinationZrc20HexOrZero.trim(); | ||
| let destinationZrc20: Buffer; | ||
| if (destRaw === '0' || /^0x?0+$/.test(destRaw)) { | ||
| destinationZrc20 = Buffer.alloc(20, 0); | ||
| } else { | ||
| const destBuf = Buffer.from(destRaw.replace(/^0x/, ''), 'hex'); | ||
| if (destBuf.length !== 20) throw new Error(`destinationZrc20 must be 20 bytes hex or '0' (got ${destBuf.length})`); | ||
| destinationZrc20 = destBuf; | ||
| } | ||
| if (!finalRecipient) throw new Error("finalRecipient must be provided as a 20-byte EVM address (hex)"); | ||
| const recRaw = finalRecipient.trim(); | ||
| const recBuf = Buffer.from(recRaw.replace(/^0x/, ''), 'hex'); | ||
| if (recBuf.length !== 20) throw new Error(`finalRecipient must be a 20-byte hex EVM address (got ${recBuf.length} bytes)`); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden hex parsing with length/charset validation.
Prevents odd-length and non-hex inputs from reaching Buffer.from.
- const tokenId = Buffer.from(tokenIdHex.replace(/^0x/, ''), 'hex');
- if (tokenId.length !== 32) throw new Error(`tokenId must be 32 bytes hex (got ${tokenId.length})`);
- const receiver = Buffer.from(zcUniversalContractHex.replace(/^0x/, ''), 'hex');
- if (receiver.length !== 20) throw new Error(`zetachain universal contract must be 20 bytes hex (got ${receiver.length})`);
+ if (!/^(0x)?[0-9a-fA-F]{64}$/.test(tokenIdHex)) throw new Error("tokenId must be 32-byte hex");
+ const tokenId = Buffer.from(tokenIdHex.replace(/^0x/, ''), 'hex');
+ if (!/^(0x)?[0-9a-fA-F]{40}$/.test(zcUniversalContractHex)) throw new Error("zetachain universal contract must be 20-byte hex");
+ const receiver = Buffer.from(zcUniversalContractHex.replace(/^0x/, ''), 'hex');
@@
- } else {
- const destBuf = Buffer.from(destRaw.replace(/^0x/, ''), 'hex');
- if (destBuf.length !== 20) throw new Error(`destinationZrc20 must be 20 bytes hex or '0' (got ${destBuf.length})`);
- destinationZrc20 = destBuf;
- }
- if (!finalRecipient) throw new Error("finalRecipient must be provided as a 20-byte EVM address (hex)");
- const recRaw = finalRecipient.trim();
- const recBuf = Buffer.from(recRaw.replace(/^0x/, ''), 'hex');
- if (recBuf.length !== 20) throw new Error(`finalRecipient must be a 20-byte hex EVM address (got ${recBuf.length} bytes)`);
+ } else {
+ if (!/^(0x)?[0-9a-fA-F]{40}$/.test(destRaw)) throw new Error("destinationZrc20 must be 20-byte hex or '0'");
+ destinationZrc20 = Buffer.from(destRaw.replace(/^0x/, ''), 'hex');
+ }
+ if (!/^(0x)?[0-9a-fA-F]{40}$/.test(finalRecipient)) throw new Error("finalRecipient must be a 20-byte EVM address (hex)");
+ const recBuf = Buffer.from(finalRecipient.replace(/^0x/, ''), 'hex');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Strict arg validations | |
| const tokenId = Buffer.from(tokenIdHex.replace(/^0x/, ''), 'hex'); | |
| if (tokenId.length !== 32) throw new Error(`tokenId must be 32 bytes hex (got ${tokenId.length})`); | |
| const receiver = Buffer.from(zcUniversalContractHex.replace(/^0x/, ''), 'hex'); | |
| if (receiver.length !== 20) throw new Error(`zetachain universal contract must be 20 bytes hex (got ${receiver.length})`); | |
| // destinationZrc20: accept '0' (stay on ZetaChain) or 20-byte hex address | |
| const destRaw = destinationZrc20HexOrZero.trim(); | |
| let destinationZrc20: Buffer; | |
| if (destRaw === '0' || /^0x?0+$/.test(destRaw)) { | |
| destinationZrc20 = Buffer.alloc(20, 0); | |
| } else { | |
| const destBuf = Buffer.from(destRaw.replace(/^0x/, ''), 'hex'); | |
| if (destBuf.length !== 20) throw new Error(`destinationZrc20 must be 20 bytes hex or '0' (got ${destBuf.length})`); | |
| destinationZrc20 = destBuf; | |
| } | |
| if (!finalRecipient) throw new Error("finalRecipient must be provided as a 20-byte EVM address (hex)"); | |
| const recRaw = finalRecipient.trim(); | |
| const recBuf = Buffer.from(recRaw.replace(/^0x/, ''), 'hex'); | |
| if (recBuf.length !== 20) throw new Error(`finalRecipient must be a 20-byte hex EVM address (got ${recBuf.length} bytes)`); | |
| // Strict arg validations | |
| if (!/^(0x)?[0-9a-fA-F]{64}$/.test(tokenIdHex)) throw new Error("tokenId must be 32-byte hex"); | |
| const tokenId = Buffer.from(tokenIdHex.replace(/^0x/, ''), 'hex'); | |
| if (!/^(0x)?[0-9a-fA-F]{40}$/.test(zcUniversalContractHex)) throw new Error("zetachain universal contract must be 20-byte hex"); | |
| const receiver = Buffer.from(zcUniversalContractHex.replace(/^0x/, ''), 'hex'); | |
| // destinationZrc20: accept '0' (stay on ZetaChain) or 20-byte hex address | |
| const destRaw = destinationZrc20HexOrZero.trim(); | |
| let destinationZrc20: Buffer; | |
| if (destRaw === '0' || /^0x?0+$/.test(destRaw)) { | |
| destinationZrc20 = Buffer.alloc(20, 0); | |
| } else { | |
| if (!/^(0x)?[0-9a-fA-F]{40}$/.test(destRaw)) throw new Error("destinationZrc20 must be 20-byte hex or '0'"); | |
| destinationZrc20 = Buffer.from(destRaw.replace(/^0x/, ''), 'hex'); | |
| } | |
| if (!/^(0x)?[0-9a-fA-F]{40}$/.test(finalRecipient)) throw new Error("finalRecipient must be a 20-byte EVM address (hex)"); | |
| const recBuf = Buffer.from(finalRecipient.replace(/^0x/, ''), 'hex'); |
🤖 Prompt for AI Agents
In contracts/nft/contracts/example/SolanaUniversalNFT.ts around lines 218 to
237, the current code calls Buffer.from on input strings without validating that
they are proper even-length hex and contain only hex characters; add explicit
validation before calling Buffer.from for tokenIdHex, zcUniversalContractHex,
destinationZrc20HexOrZero (when not '0'), and finalRecipient: strip an optional
0x prefix, ensure the remaining string length is even and matches
/^[0-9a-fA-F]+$/ (for destination allow a single '0' or all-zero pattern handled
already), and throw clear errors indicating invalid charset or odd length; only
then convert to Buffer and continue with the existing length checks and error
messages.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
Yes, it's completely fine to change the relayer (I think you mean ZetaChain Universal NFT) as long as it keeps working with EVM chains. @kingpinXD wdyt? |
|
Either of the two options is fine What do you mean by modifying the relayer? The |
| // 3) Mint 1 to program_ata using config PDA as authority | ||
| mint_nft_to_recipient( | ||
| &ctx.accounts.mint_account, | ||
| &ctx.accounts.pda_ata, |
There was a problem hiding this comment.
How does this get transferred to the user who initiated the call
There was a problem hiding this comment.
i guess its wrong. i have to change this. i assume the recipient address will be in data payload so i could get recipient ata from the recipient wallet address and mint, then transfer the nft in pda_ata to that ata.
There was a problem hiding this comment.
i pushed the changes and now used token::transfer_checked to transfer nft from pda_ata to recipient_ata. but i added recipient and recipient_ata accounts into the OnCall context instead of getting them through ctx.remaining_accounts bc i had an annoying lifetime issue during the build.
There was a problem hiding this comment.
what is the issue you were facing ?
Its not super clear to me what you mean by not using remianing accounts,
the account_metas for the on_call is created like so
…a to recipient_ata
|
Please add the following to your PR: Contract addresses on Base Sepolia, ZetaChain, Solana Devnet Tx hash: Solana → Base Tx hash: Base → Solana Tx hash: ZetaChain → Solana These are required before we proceed with review. |
|
idk because i spammed the transfer too much but i encountered this error several times during an NFT transfer from Base Sepolia to Solana. Base Sepolia -> Zetachain: https://zetachain-athens.blockpi.network/lcd/v1/public/zeta-chain/crosschain/inboundHashToCctxData/0x1fa2bb603780ab9965e881de8268f3c1403d46416119c36468df2358e07318c6 Zetachain -> Solana: https://zetachain-athens.blockpi.network/lcd/v1/public/zeta-chain/crosschain/inboundHashToCctxData/0xa2b2096c5461930e45e8506913b7d69db62c28890f1a300f219b10755be51bdb i even tried with an empty/example (one of the zetachain solana examples on github) on_call, always got tx outbound failed on the connected chain error. after a while, it started failing on the Zetachain side too xd. to keep here, My contract addresses: |
|
Base Sepolia -> Zetachain: This transaction seems okay Zetachain -> Solana: This tx reverted from the solana program, and went back to Zetachain. The revert was however, successful. Is this tx the same as the first one ? Were you able to use the revert reason to figure out the reason ? |
|
Please check your messages we are ready to announce a winner from the Superteam Earn bounty. |
|
Hey thanks for the notice, i was logged out somehow and couldnt see the announcement. It was a pleasure for me to work on it. I couldnt make the nft transfer to work from other chains to Solana properly though. The problem was on my solana side and while i was about to debug it, i faced with another issue that i mentioned which was quite annoying for me. Sometimes increasing the gas amount to 0.02 - 0.03 during the nft transfer from base sepolia to solana was solving that though. The solution for my problem is probably quite easy but couldnt make it so far. I would be happy if you have time for a quick meet/zoom call in this week so that i can show my current status. Finally i would like to make this program working completely. Best Regards
|
5 participants
Universal NFT Program (Solana)
This PR introduces a secure and deterministic Universal NFT program on Solana using Anchor, with ZetaChain Gateway integration for cross-chain transfers.
Overview
on_call.Accounts and PDAs
Instructions (public API)
initialize(gateway_program)
reserve_next_token_id()
mint_nft(name, symbol, uri)
transfer_to_zetachain(token_id[32], receiver[20], destination_chain u64, final_recipient string)
callwith ABI-encoded message; emits CrossChainTransferInitiated and NftBurned.on_call(amount u64, sender[20], data Vec)
update_config(new_authority?, new_gateway_program?, new_gateway_pda?, pause?)
Cross-chain flow
Security and invariants
Events
Client notes (TS example)
Summary by CodeRabbit
New Features
Chores