Conversation
WalkthroughThis update introduces comprehensive integration of the Anchor framework for Solana smart contract interactions, specifically targeting escrow operations. New Anchor-based services, utilities, and parsing modules are added, while legacy IDL and type files are removed. Module providers and service dependencies are updated to support the Anchor workflow, and detailed refactoring documentation is included. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant EscrowAnchorService
participant AnchorService
participant AnchorProgram
participant SolanaNetwork
Client->>EscrowAnchorService: fundEscrow(contractId, signer, amount)
EscrowAnchorService->>AnchorService: getEscrowProgram(wallet)
AnchorService-->>EscrowAnchorService: AnchorProgram instance
EscrowAnchorService->>AnchorProgram: Build transaction (fundEscrow)
EscrowAnchorService->>EscrowAnchorService: Serialize transaction, queue for signing
EscrowAnchorService-->>Client: ApiResponse (tx queued)
Client->>EscrowAnchorService: getEscrowByContractID(signer, contractId)
EscrowAnchorService->>AnchorService: getEscrowProgram()
AnchorService-->>EscrowAnchorService: AnchorProgram instance
EscrowAnchorService->>AnchorProgram: Fetch escrow account data
EscrowAnchorService-->>Client: EscrowCamelCaseResponse
Client->>EscrowAnchorService: changeDisputeFlag(contractId, signer)
EscrowAnchorService->>AnchorService: getEscrowProgram(wallet)
AnchorService-->>EscrowAnchorService: AnchorProgram instance
EscrowAnchorService->>AnchorProgram: Build transaction (changeDisputeFlag)
EscrowAnchorService->>EscrowAnchorService: Serialize transaction, queue for signing
EscrowAnchorService-->>Client: ApiResponse (tx queued)
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/server/src/app.module.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/apps/server".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "apps/server/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. apps/server/src/solana-contract/escrow/escrow-anchor.service.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/apps/server".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "apps/server/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. apps/server/src/solana-contract/escrow/escrow.module.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/apps/server".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "apps/server/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
these should be restored 👀
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the escrow API integration by replacing Borsh-based serialization with Anchor’s fluent interface, as well as introducing new utilities and services to support Anchor transactions. Key changes include updating test imports, creating new Anchor utility modules, and adding an Anchor-powered escrow service.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/smart-contract/tests/escrow.test.ts | Adjusted import ordering for type-only imports |
| apps/server/src/utils/transaction-anchor.utils.ts | Introduced Anchor transaction building and serialization utilities |
| apps/server/src/utils/parse.utils.ts | Added AnchorEscrowAccount interface and conversion function |
| apps/server/src/utils/parse-anchor.utils.ts | Created additional parsing and conversion utilities for Anchor |
| apps/server/src/utils/anchor.utils.ts | Implemented utility functions for creating and managing Anchor programs |
| apps/server/src/solana-contract/anchor/anchor.service.ts | New service for managing Anchor program interactions |
| apps/server/src/solana-contract/escrow/escrow-anchor.service.ts | New Anchor-powered escrow service with several transaction builders |
| apps/server/src/solana-contract/escrow/escrow.module.ts | Updated module configuration to register Anchor-based services |
| ANCHOR_REFACTORING_GUIDE.md | Added documentation detailing the refactoring approach and examples |
| try { | ||
| // The IDL is available in the @programs/solana-tl package | ||
| const { EscrowIDL } = require('@programs/solana-tl') | ||
| this._escrowIdl = EscrowIDL |
There was a problem hiding this comment.
[nitpick] Consider providing an explicit type for _escrowIdl instead of using 'any' to improve type-safety and maintainability.
| * Convert an EscrowDto to an Anchor-compatible escrow data structure | ||
| * The structure should match the Rust struct in your Solana program | ||
| */ | ||
| export function escrowDtoToAnchorData(dto: any): any { |
There was a problem hiding this comment.
[nitpick] Consider defining specific types for both the input DTO and the returned Anchor data structure to ensure consistency and clarity in the API design.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (3)
apps/server/src/solana-contract/escrow/escrow.service.ts (2)
516-594:⚠️ Potential issueIncomplete migration in updateEscrowByContractID.
The updateEscrowByContractID method references
this.programand uses Anchor-style methods, butprogramis not defined in the class and several undefined functions are used (SolanaAcc, parseEscrowDtoToSolanaEscrowData).This method needs to be fully updated to utilize the newly injected AnchorService. Implement the missing components:
async updateEscrowByContractID( contractId: string, signer: string, escrow: EscrowDto, ): Promise<ApiResponse> { try { const platformSignerPubkey = new PublicKey(signer) - const escrowAccountPubkey = SolanaAcc(contractId) + const escrowAccountPubkey = new PublicKey(contractId) // Parse the DTO into the data structure expected by the Solana program // Ensure EscrowDto has all necessary fields as per EscrowStructure/SolanaEscrowData - const solanaEscrowData = parseEscrowDtoToSolanaEscrowData(escrow as any) + // Use appropriate parsing function or directly map the DTO + const solanaEscrowData = escrow // Or map to the expected format - const instruction = await this.program.methods + const instruction = await this.anchorService.program.methods .changeEscrowProperties(solanaEscrowData) .accounts({ platform: platformSignerPubkey, escrowAccount: escrowAccountPubkey, systemProgram: SystemProgram.programId, }) .instruction()
205-213: 💡 Verification agent🧩 Analysis chain
Plans for future migration.
These comments indicate that the methods below still need to be updated from Stellar to Solana implementations. Since this is marked as WIP in the PR title, this is expected, but a clear migration plan should be established.
Consider creating tickets or tasks to track the migration of each method. For now, let's check which methods have been migrated to the AnchorService:
🏁 Script executed:
#!/bin/bash # Find the EscrowAnchorService implementation to see which methods have been migrated fd -t f "escrow-anchor.service.ts" apps/server/src/solana-contract/escrow/ cat apps/server/src/solana-contract/escrow/escrow-anchor.service.tsLength of output: 9777
Track and Complete Solana Migration for Escrow Service
The
escrow.service.tsfile still contains a big “Stellar-specific” stub comment, while the actual Solana implementations have been moved intoEscrowAnchorService. Let’s catalog what’s already in place and plan the outstanding work.• Migrated methods in
EscrowAnchorService
- fundEscrow
- changeDisputeFlag
- updateEscrowByContractID
- distributeEscrowEarnings
- getEscrowByContractID
• Remaining work in
escrow.service.ts
- Remove or replace Stellar-specific methods below the stub comment
- Delegate each operation to the corresponding Anchor service method
Action items:
- Create tickets/tasks for each Stellar method to be replaced, e.g.:
- Migrate
createEscrowAccount- Migrate
refundEscrow- …
- Update
escrow.service.ts:
- Remove the big block comment once all methods are migrated
- Wire up each endpoint to
EscrowAnchorServicePlease track these migrations in your issue tracker so nothing slips through.
apps/server/src/utils/parse.utils.ts (1)
273-283:⚠️ Potential issueIncorrect micro-unit conversion
adjustPricesToMicroUSDCmultiplies bydecimalsinstead of10 ** decimals.
For USDC (6 decimals) a price of1.23currently yields7.38– clearly wrong.- const microStable = BigInt( - Math.round(Number.parseFloat(price.toString()) * decimals), - ) + const factor = 10n ** BigInt(decimals) + const microStable = BigInt(Math.round(Number(price) * 1e6)) * (factor / 1_000_000n)Likewise,
microUSDTToDecimalmust divide by10 ** decimals, notdecimals.
🧹 Nitpick comments (11)
apps/server/src/utils/parse.utils.ts (1)
256-271:fetchAnchorEscrowAccountignores wallet context
program.account.escrowAccount.fetchwill fail on private or PDA accounts that require an authenticated RPC connection if the provider haswallet = null.Expose an overload that forwards a wallet (or pre-created provider) to
AnchorService.getEscrowProgram()to avoid silent runtime failures once such accounts are introduced.apps/server/src/solana-contract/escrow/escrow-anchor.service.ts (2)
38-39: Unused field
this.solanaServer = this.helperService.getServerConnection()
solanaServeris never read thereafter → dead code. Remove to keep the service lean.
321-325:signerparameter is unused
getEscrowByContractID(signer, contractId)never referencessigner.
Either remove the argument or forward it (e.g., toAnchorService.getEscrowProgram(wallet)), otherwise callers may assume access control that isn’t enforced.apps/server/src/utils/parse-anchor.utils.ts (2)
59-70: Consider enriching response with additional metadataWhile the conversion handles the basic fields, consider whether additional metadata should be included in the response such as timestamps, transaction IDs, or status details that might be present in the actual Anchor account structure.
114-129: Add handling for negative amounts in fromMicroUSDCThe current implementation doesn't explicitly handle negative BN values. While Solana tokens typically don't use negative values, having this handling would make the function more robust.
apps/server/src/utils/transaction-anchor.utils.ts (1)
1-1: Use the node: protocol for Node.js builtin modulesFollow the recommended practice of using the node: protocol for Node.js built-in modules.
-import { createHash } from 'crypto' +import { createHash } from 'node:crypto'🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
ANCHOR_REFACTORING_GUIDE.md (3)
9-11: Update dependency referenceThe guide references
@project-serum/anchorwhich is outdated. The codebase is using@coral-xyz/anchorwhich is the updated package name.```bash -npm install @coral-xyz/anchor @project-serum/anchor +npm install @coral-xyz/anchor--- `285-299`: **Ensure toMicroUSDC code sample matches implementation** The code sample for `toMicroUSDC` in the guide has slight differences from the actual implementation in `parse-anchor.utils.ts`, particularly in the variable declaration style (using `let` vs. `const`). Consider updating the guide to match the actual implementation: ```diff export function toMicroUSDC( amount: string | number, decimals: number = 6, ): anchor.BN { const amountStr = amount.toString(); const [integerPart, fractionalPart = ""] = amountStr.split("."); - let paddedFractional = fractionalPart + const paddedFractional = fractionalPart .padEnd(decimals, "0") .slice(0, decimals); const microAmount = integerPart + paddedFractional; return new anchor.BN(microAmount); }
301-308: Expand Next Steps with testing guidanceThe guide helpfully outlines next steps for the refactoring process, but could benefit from more specific testing recommendations.
Consider adding details about:
- Which specific test frameworks to use with Anchor (e.g., Anchor's own testing framework)
- How to mock Anchor program interactions in unit tests
- Strategies for integration testing with a local validator
🧰 Tools
🪛 LanguageTool
[grammar] ~304-~304: The verb “Consider” needs to be in the to-infinitive form.
Context: ...to inject the EscrowAnchorService where needed 3. Consider a phased approach where you migrate one...(MISSING_TO_BEFORE_A_VERB)
apps/server/src/utils/anchor.utils.ts (2)
1-1: Use the node: protocol for Node.js builtin modulesFollow the recommended practice of using the node: protocol for Node.js built-in modules.
-import { createHash } from 'crypto' +import { createHash } from 'node:crypto'🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
17-19: Use dynamic import() instead of requireThe codebase is using ES modules syntax, but this function uses CommonJS
require(). For consistency and better error handling, consider using dynamicimport()instead.- // Import the IDL dynamically - // We're assuming the IDL is available in the programs/solana-tl package - const { EscrowIDL } = require('@programs/solana-tl') + // Import the IDL dynamically + // We're assuming the IDL is available in the programs/solana-tl package + const { EscrowIDL } = await import('@programs/solana-tl')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (16)
ANCHOR_REFACTORING_GUIDE.md(1 hunks)apps/server/src/app.module.ts(1 hunks)apps/server/src/solana-contract/anchor/anchor.service.ts(1 hunks)apps/server/src/solana-contract/escrow/escrow-anchor.service.ts(1 hunks)apps/server/src/solana-contract/escrow/escrow.module.ts(1 hunks)apps/server/src/solana-contract/escrow/escrow.service.ts(2 hunks)apps/server/src/solana-contract/solana-contract.module.ts(1 hunks)apps/server/src/utils/anchor.utils.ts(1 hunks)apps/server/src/utils/parse-anchor.utils.ts(1 hunks)apps/server/src/utils/parse.utils.ts(2 hunks)apps/server/src/utils/transaction-anchor.utils.ts(1 hunks)apps/smart-contract/tests/escrow.test.ts(1 hunks)programs/solana-tl/target/idl/escrow.json(0 hunks)programs/solana-tl/target/idl/token.json(0 hunks)programs/solana-tl/target/types/escrow.ts(0 hunks)programs/solana-tl/target/types/token.ts(0 hunks)
💤 Files with no reviewable changes (4)
- programs/solana-tl/target/types/escrow.ts
- programs/solana-tl/target/types/token.ts
- programs/solana-tl/target/idl/token.json
- programs/solana-tl/target/idl/escrow.json
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/server/src/solana-contract/solana-contract.module.ts (3)
apps/server/src/solana-contract/escrow/escrow.module.ts (1)
Module(12-23)apps/server/src/solana-contract/deployer/deployer.module.ts (1)
Module(7-12)apps/server/src/solana-contract/helper/helper.module.ts (1)
Module(10-20)
apps/server/src/utils/parse.utils.ts (2)
apps/server/src/interfaces/escrow.interface.ts (1)
Escrow(4-25)apps/server/src/utils/anchor.utils.ts (1)
bnToString(198-201)
🪛 Biome (1.9.4)
apps/server/src/utils/transaction-anchor.utils.ts
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
apps/server/src/utils/anchor.utils.ts
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
🪛 LanguageTool
ANCHOR_REFACTORING_GUIDE.md
[grammar] ~304-~304: The verb “Consider” needs to be in the to-infinitive form.
Context: ...to inject the EscrowAnchorService where needed 3. Consider a phased approach where you migrate one...
(MISSING_TO_BEFORE_A_VERB)
🔇 Additional comments (8)
apps/server/src/app.module.ts (1)
30-30: Approve trivial formatting adjustment
Removed the extra space inside the class braces for consistency with project style; no functional changes introduced.apps/smart-contract/tests/escrow.test.ts (1)
2-2: Import reordering for type usage.The import statement has been reordered to explicitly import
Escrowas a type. This reflects the ongoing migration from raw Borsh serialization to the Anchor framework by indicating thatEscrowis now used only as a type rather than a class/object.apps/server/src/solana-contract/solana-contract.module.ts (1)
2-2: Integration of AnchorService for DI system.The AnchorService is properly registered as both a provider and export, making it available throughout the dependency injection system. This change supports the migration from Borsh to Anchor framework for Solana smart contract interactions.
Also applies to: 10-11
apps/server/src/solana-contract/escrow/escrow.service.ts (1)
21-21: Dependency injection of AnchorService.The AnchorService is now injected into EscrowService, preparing for the migration to Anchor-based operations. This is a good step in the refactoring process.
Also applies to: 48-48
apps/server/src/solana-contract/escrow/escrow.module.ts (1)
4-4: Integration of Anchor services.The module has been updated to provide and export both EscrowAnchorService and AnchorService along with the existing services. This approach allows for a phased migration, enabling parts of the application to use either the legacy implementation or the new Anchor-based services.
Also applies to: 7-7, 15-21
apps/server/src/utils/parse.utils.ts (1)
224-230: Milestone key naming mismatchYou convert
approvedFlag→approved_flag, butMilestoneinEscrow(seeinterfaces/escrow.interface.ts) already storesapproved_flag.
If the on-chain struct is camelCase (approvedFlag) you’ll lose parity; if it’s snake case the current mapping is fine but the interface should match.Please verify schema alignment and adjust either the interface or the mapper.
apps/server/src/solana-contract/escrow/escrow-anchor.service.ts (1)
193-196: Fallback token account may break program expectationsWhen
escrow.tokenAccountis undefined you fall back toescrowAccountPubkey, but programs usually expect an ATA, not the escrow PDA itself.Double-check the program’s
accountssection; passing the wrong account will cause instruction failure.apps/server/src/utils/parse-anchor.utils.ts (1)
43-54:Details
❓ Verification inconclusive
Finalize the escrowDtoToAnchorData implementation based on your contract's structure
The function is currently structured as a template with a comment indicating it should be customized. Ensure that all fields from your Rust struct are properly mapped here to avoid serialization issues when interacting with the Solana program.
🏁 Script executed:
#!/bin/bash # Check the actual escrow account structure in Rust to ensure mapping is complete fd -e rs -x grep -A 20 "pub struct Escrow" {}Length of output: 747
Unable to locate the on-chain Escrow struct
I searched for a#[account] pub struct Escrow…definition but couldn’t find it in the Rust code. Please confirm the exact name, file path and full field list of your Escrow account (or instruction args) so we can ensureescrowDtoToAnchorDatain
apps/server/src/utils/parse-anchor.utils.ts:43-54
correctly maps every property and avoids any serialization mismatches.
| getProvider(wallet?: anchor.Wallet): anchor.AnchorProvider { | ||
| return new anchor.AnchorProvider(this.connection, wallet || null, { | ||
| commitment: 'confirmed', | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
wallet may not be optional for mutating calls
new anchor.AnchorProvider(this.connection, wallet || null, …) creates a provider with a null wallet.
As soon as you invoke any RPC that needs a signer (all mutation flows in EscrowAnchorService), Anchor will throw “provider.wallet is null”.
Two options:
- Require the caller to supply a wallet when a signed tx is needed.
- Lazily create a read-only “dummy” provider only for
.fetch()operations and expose a separate method for signed providers.
Clarifying this now prevents difficult‐to-trace runtime failures.
🤖 Prompt for AI Agents
In apps/server/src/solana-contract/anchor/anchor.service.ts around lines 32 to
36, the getProvider method currently allows creating an AnchorProvider with a
null wallet, which causes runtime errors when signing is required. To fix this,
either make the wallet parameter mandatory for all calls that require signing or
separate the logic by creating one method that returns a read-only provider with
no wallet for fetch-only operations and another method that requires a wallet
for signed transactions. This ensures that mutation calls always have a valid
wallet and prevents null wallet errors.
| getEscrowProgram(wallet?: anchor.Wallet): Program { | ||
| const provider = this.getProvider(wallet) | ||
| return new anchor.Program(this._escrowIdl, this.escrowProgramId, provider) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Type safety & early failure for _escrowIdl
_escrowIdl is typed as any; the compiler cannot help when the wrong IDL shape is supplied.
Declare a concrete type (anchor.Idl) and assert non-null before instantiating the program:
- private _escrowIdl: any
+ private _escrowIdl!: anchor.Idland
if (!this._escrowIdl)
throw new Error('Escrow IDL is undefined');📝 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.
| getEscrowProgram(wallet?: anchor.Wallet): Program { | |
| const provider = this.getProvider(wallet) | |
| return new anchor.Program(this._escrowIdl, this.escrowProgramId, provider) | |
| } | |
| // In apps/server/src/solana-contract/anchor/anchor.service.ts | |
| export class AnchorService { | |
| // … | |
| - private _escrowIdl: any; | |
| + private _escrowIdl!: anchor.Idl; | |
| getEscrowProgram(wallet?: anchor.Wallet): Program { | |
| const provider = this.getProvider(wallet); | |
| + if (!this._escrowIdl) { | |
| + throw new Error('Escrow IDL is undefined'); | |
| + } | |
| return new anchor.Program( | |
| this._escrowIdl, | |
| this.escrowProgramId, | |
| provider, | |
| ); | |
| } | |
| // … | |
| } |
🤖 Prompt for AI Agents
In apps/server/src/solana-contract/anchor/anchor.service.ts around lines 49 to
52, the _escrowIdl property is typed as any, which prevents type safety and
early error detection. Change the type of _escrowIdl to anchor.Idl to enforce
the correct IDL shape, and add a non-null assertion or explicit check before
using it to instantiate the Program, ensuring the value is valid and preventing
runtime errors.
| try { | ||
| // The IDL is available in the @programs/solana-tl package | ||
| const { EscrowIDL } = require('@programs/solana-tl') | ||
| this._escrowIdl = EscrowIDL | ||
| } catch (error) { | ||
| console.error('Failed to load IDL from package:', error) | ||
| } | ||
| } |
There was a problem hiding this comment.
Guard against missing or invalid IDL
require('@programs/solana-tl') is wrapped in a try / catch, but if the IDL fails to load _escrowIdl remains undefined.
Every later call to new anchor.Program(this._escrowIdl, …) will then crash at runtime.
Add an explicit guard (or re-throw) so the service fails fast instead of propagating a cryptic error much later.
- const { EscrowIDL } = require('@programs/solana-tl')
- this._escrowIdl = EscrowIDL
+ const { EscrowIDL } = require('@programs/solana-tl')
+ if (!EscrowIDL) throw new Error('EscrowIDL not found in package')
+ this._escrowIdl = EscrowIDL📝 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.
| try { | |
| // The IDL is available in the @programs/solana-tl package | |
| const { EscrowIDL } = require('@programs/solana-tl') | |
| this._escrowIdl = EscrowIDL | |
| } catch (error) { | |
| console.error('Failed to load IDL from package:', error) | |
| } | |
| } | |
| try { | |
| // The IDL is available in the @programs/solana-tl package | |
| const { EscrowIDL } = require('@programs/solana-tl') | |
| if (!EscrowIDL) throw new Error('EscrowIDL not found in package') | |
| this._escrowIdl = EscrowIDL | |
| } catch (error) { | |
| console.error('Failed to load IDL from package:', error) | |
| } |
🤖 Prompt for AI Agents
In apps/server/src/solana-contract/anchor/anchor.service.ts around lines 20 to
27, the code catches errors when requiring the IDL but does not handle the case
where _escrowIdl remains undefined, leading to runtime crashes later. To fix
this, after the try/catch block, add an explicit check to verify that _escrowIdl
is defined; if not, throw a clear error or re-throw the caught error to fail
fast and prevent later cryptic failures when using the IDL.
| amount: Number(bnToString(anchorAccount.amount)), | ||
| approver: anchorAccount.approver?.toString() || '', | ||
| serviceProvider: anchorAccount.serviceProvider?.toString() || '', | ||
| platformAddress: anchorAccount.platformAddress?.toString() || '', | ||
| platformFee: Number(bnToString(anchorAccount.platformFee)), | ||
| releaseSigner: anchorAccount.releaseSigner?.toString() || '', | ||
| disputeResolver: anchorAccount.disputeResolver?.toString() || '', | ||
| trustline: anchorAccount.trustline?.toString() || '', | ||
| receiver: anchorAccount.receiver?.toString() || '', | ||
| trustlineDecimals: Number(bnToString(anchorAccount.trustlineDecimals)), | ||
| receiverMemo: Number(bnToString(anchorAccount.receiverMemo)), | ||
| disputeFlag: !!anchorAccount.disputeFlag, |
There was a problem hiding this comment.
BN → Number loses precision for on-chain amounts
Number(bnToString(anchorAccount.amount)) (and similar conversions) will truncate values ≥ 2⁵³-1.
Amounts, fees, timestamps, etc. routinely exceed that threshold.
Prefer returning strings (or bigint) to callers and let the UI / service layer decide how to format.
- amount: Number(bnToString(anchorAccount.amount)),
+ amount: bnToString(anchorAccount.amount), // keep full precisionRepeat for every large-integer field.
📝 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.
| amount: Number(bnToString(anchorAccount.amount)), | |
| approver: anchorAccount.approver?.toString() || '', | |
| serviceProvider: anchorAccount.serviceProvider?.toString() || '', | |
| platformAddress: anchorAccount.platformAddress?.toString() || '', | |
| platformFee: Number(bnToString(anchorAccount.platformFee)), | |
| releaseSigner: anchorAccount.releaseSigner?.toString() || '', | |
| disputeResolver: anchorAccount.disputeResolver?.toString() || '', | |
| trustline: anchorAccount.trustline?.toString() || '', | |
| receiver: anchorAccount.receiver?.toString() || '', | |
| trustlineDecimals: Number(bnToString(anchorAccount.trustlineDecimals)), | |
| receiverMemo: Number(bnToString(anchorAccount.receiverMemo)), | |
| disputeFlag: !!anchorAccount.disputeFlag, | |
| amount: bnToString(anchorAccount.amount), // keep full precision | |
| approver: anchorAccount.approver?.toString() || '', | |
| serviceProvider: anchorAccount.serviceProvider?.toString() || '', | |
| platformAddress: anchorAccount.platformAddress?.toString() || '', | |
| platformFee: Number(bnToString(anchorAccount.platformFee)), | |
| releaseSigner: anchorAccount.releaseSigner?.toString() || '', | |
| disputeResolver: anchorAccount.disputeResolver?.toString() || '', | |
| trustline: anchorAccount.trustline?.toString() || '', | |
| receiver: anchorAccount.receiver?.toString() || '', | |
| trustlineDecimals: Number(bnToString(anchorAccount.trustlineDecimals)), | |
| receiverMemo: Number(bnToString(anchorAccount.receiverMemo)), | |
| disputeFlag: !!anchorAccount.disputeFlag, |
🤖 Prompt for AI Agents
In apps/server/src/utils/parse.utils.ts around lines 209 to 220, the current
code converts large BN values to Number, which causes precision loss for values
exceeding 2^53-1. To fix this, change all conversions from
Number(bnToString(...)) to just return the string representation
(bnToString(...)) or convert to bigint if preferred. This applies to all
large-integer fields like amount, platformFee, trustlineDecimals, receiverMemo,
etc. Ensure the returned values are strings or bigints so the UI or service
layer can handle formatting without precision loss.
| async getEscrowByContractID( | ||
| signer: string, | ||
| contractId: string, | ||
| ): Promise<EscrowCamelCaseResponse> { | ||
| try { | ||
| // Convert address to PublicKey | ||
| const escrowAccountPubkey = new PublicKey(contractId) | ||
|
|
||
| // Get the Anchor program | ||
| const program = this.anchorService.getEscrowProgram() | ||
|
|
||
| // Fetch the account data using Anchor's typed fetching | ||
| const escrowAccount = | ||
| await program.account.escrowAccount.fetch(escrowAccountPubkey) | ||
|
|
||
| // Convert Anchor account data to your API response type | ||
| // The exact mapping depends on your specific Anchor IDL and response type | ||
| return { | ||
| approver: escrowAccount.approver.toString(), | ||
| receiver: escrowAccount.receiver.toString(), | ||
| amount: fromBN(escrowAccount.amount), | ||
| title: escrowAccount.title || '', | ||
| description: escrowAccount.description || '', | ||
| status: escrowAccount.status || 'PENDING', | ||
| disputeFlag: escrowAccount.disputeFlag || false, | ||
| // Map other fields from escrowAccount to your EscrowCamelCaseResponse type | ||
| // This is just an example and needs to be adjusted to your specific data structures | ||
| trustlineDecimals: 6, // Default for USDC | ||
| } |
There was a problem hiding this comment.
Returned object violates EscrowCamelCaseResponse
EscrowCamelCaseResponse includes many mandatory fields (e.g. platformFee, receiver, trustline, …) that are missing here.
This will either fail TS compilation or surface as undefined at runtime.
Populate all fields or relax the interface.
🤖 Prompt for AI Agents
In apps/server/src/solana-contract/escrow/escrow-anchor.service.ts around lines
303 to 331, the returned object from getEscrowByContractID is missing several
mandatory fields defined in the EscrowCamelCaseResponse interface, such as
platformFee, receiver, and trustline. To fix this, ensure that all required
fields in EscrowCamelCaseResponse are populated with appropriate values from
escrowAccount or default values before returning the object. Alternatively, if
some fields are optional, update the interface to reflect that, but the
preferred fix is to fully populate the response object to match the interface.
| export function toBN(amount: string | number): anchor.BN { | ||
| try { | ||
| return new anchor.BN(amount) | ||
| } catch (error) { | ||
| console.error('Error converting to BN:', error) | ||
| return new anchor.BN(0) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate BN conversion utilities with anchor.utils.ts
Both this file and anchor.utils.ts contain similar functions for BN conversion. Consider consolidating these utilities in one location to avoid duplication.
// Remove these functions and import from anchor.utils.ts instead
-export function toBN(amount: string | number): anchor.BN {
- try {
- return new anchor.BN(amount)
- } catch (error) {
- console.error('Error converting to BN:', error)
- return new anchor.BN(0)
- }
-}
-
-export function fromBN(bn: anchor.BN | null | undefined): string {
- if (!bn) return '0'
- return bn.toString()
-}
// Import them instead:
+import { bnToString as fromBN, stringToBN as toBN } from './anchor.utils'Also applies to: 146-149
🤖 Prompt for AI Agents
In apps/server/src/utils/transaction-anchor.utils.ts around lines 134 to 141 and
also lines 146 to 149, there are duplicate BN conversion functions similar to
those in anchor.utils.ts. To fix this, remove the BN conversion functions from
transaction-anchor.utils.ts and instead import and use the existing BN
conversion utilities from anchor.utils.ts to consolidate the code and avoid
duplication.
| export async function buildAnchorTransaction({ | ||
| program, | ||
| methodName, | ||
| args = [], | ||
| accounts, | ||
| feePayer, | ||
| }: { | ||
| program: Program | ||
| methodName: string | ||
| args?: any[] | ||
| accounts: Record<string, PublicKey> | ||
| feePayer: PublicKey | ||
| }): Promise<{ | ||
| transaction: Transaction | ||
| serializedTransaction: string | ||
| txId: string | ||
| }> { | ||
| try { | ||
| if (!program.methods[methodName]) { | ||
| throw new Error(`Method "${methodName}" not found in program`) | ||
| } | ||
|
|
||
| // Build transaction using Anchor's fluent interface | ||
| const tx = await program.methods[methodName](...args) | ||
| .accounts(accounts) | ||
| .transaction() | ||
|
|
||
| // Set fee payer | ||
| tx.feePayer = feePayer | ||
|
|
||
| // Get recent blockhash | ||
| const { blockhash } = | ||
| await program.provider.connection.getLatestBlockhash('confirmed') | ||
| tx.recentBlockhash = blockhash | ||
|
|
||
| // Serialize transaction for client-side signing | ||
| const serializedTx = tx.serialize({ | ||
| requireAllSignatures: false, | ||
| verifySignatures: false, | ||
| }) | ||
|
|
||
| // Create transaction ID hash for tracking | ||
| const txId = createHash('sha256').update(serializedTx).digest('hex') | ||
|
|
||
| return { | ||
| transaction: tx, | ||
| serializedTransaction: serializedTx.toString('base64'), | ||
| txId, | ||
| } | ||
| } catch (error) { | ||
| console.error('Error building Anchor transaction:', error) | ||
|
|
||
| let errorMessage = 'Failed to build transaction' | ||
| if (error instanceof Error) { | ||
| errorMessage = error.message | ||
| } | ||
|
|
||
| throw new HttpException( | ||
| { status: HttpStatus.BAD_REQUEST, message: errorMessage }, | ||
| HttpStatus.BAD_REQUEST, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate transaction building with anchor.utils.ts
There appears to be significant overlap between this buildAnchorTransaction function and createUnsignedAnchorTransaction in anchor.utils.ts. Consider consolidating these functions to avoid duplication.
Maintain a single source of truth for transaction building logic to improve maintainability. Consider keeping this function and updating references to createUnsignedAnchorTransaction to use this implementation instead.
🤖 Prompt for AI Agents
In apps/server/src/utils/transaction-anchor.utils.ts from lines 16 to 78, the
buildAnchorTransaction function duplicates logic found in
createUnsignedAnchorTransaction in anchor.utils.ts. To fix this, refactor by
consolidating the transaction building logic into a single function, preferably
keeping buildAnchorTransaction here. Then update all references to
createUnsignedAnchorTransaction to use buildAnchorTransaction instead, ensuring
only one source of truth for transaction creation to improve maintainability and
reduce code duplication.
| /** | ||
| * Fetch and deserialize an account using Anchor's typed account fetching | ||
| */ | ||
| export async function fetchAnchorAccount<T>({ | ||
| program, | ||
| accountAddress, | ||
| accountType, | ||
| }: { | ||
| program: Program | ||
| accountAddress: PublicKey | ||
| accountType: string | ||
| }): Promise<T> { | ||
| try { | ||
| if (!program.account[accountType]) { | ||
| throw new Error(`Account type "${accountType}" not found on program`) | ||
| } | ||
|
|
||
| return await program.account[accountType].fetch(accountAddress) | ||
| } catch (error) { | ||
| console.error(`Error fetching ${accountType} account:`, error) | ||
|
|
||
| let errorMessage = `Failed to fetch ${accountType} account` | ||
| if (error instanceof Error) { | ||
| errorMessage = error.message | ||
| } | ||
|
|
||
| throw new HttpException( | ||
| { status: HttpStatus.BAD_REQUEST, message: errorMessage }, | ||
| HttpStatus.BAD_REQUEST, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure consistent naming with parseAnchorAccount
The function fetchAnchorAccount does the same thing as parseAnchorAccount in parse-anchor.utils.ts but with different parameter structure. Consider consolidating these functions or ensuring they're distinctly different in purpose.
To avoid confusion, either:
- Consolidate these functions to a single implementation
- Rename one of them to better reflect its specific purpose
- Add comments to clarify when to use each function
🤖 Prompt for AI Agents
In apps/server/src/utils/anchor.utils.ts around lines 162 to 193, the function
fetchAnchorAccount duplicates functionality found in parseAnchorAccount in
parse-anchor.utils.ts but uses a different parameter structure. To fix this,
either consolidate both functions into a single implementation to avoid
redundancy, or rename one function to clearly indicate its distinct purpose.
Additionally, add comments explaining the intended use cases for each function
if they must remain separate, ensuring clarity and preventing confusion.
| if (error.logs) { | ||
| errorMessage += ` Program logs: ${error.logs.join(', ')}` | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null/undefined check before accessing error.logs
Similar to the previous error handling issue, the code accesses error.logs directly without proper validation.
- if (error.logs) {
+ if (error && typeof error === 'object' && 'logs' in error) {
errorMessage += ` Program logs: ${error.logs.join(', ')}`
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/server/src/utils/anchor.utils.ts around lines 151 to 153, the code
accesses error.logs directly without checking if error.logs is null or
undefined. Add a null/undefined check before accessing error.logs to ensure the
code only attempts to join and append logs if error.logs exists and is an array.
| if (error.logs) { | ||
| errorMessage += ` Program logs: ${error.logs.join(', ')}` | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null/undefined check before accessing error.logs
The code accesses error.logs directly without verifying if error is an object with this property, which could result in a runtime error if error is a primitive value.
- if (error.logs) {
+ if (error && typeof error === 'object' && 'logs' in error) {
errorMessage += ` Program logs: ${error.logs.join(', ')}`
}📝 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.
| if (error.logs) { | |
| errorMessage += ` Program logs: ${error.logs.join(', ')}` | |
| } | |
| if (error && typeof error === 'object' && 'logs' in error) { | |
| errorMessage += ` Program logs: ${error.logs.join(', ')}` | |
| } |
🤖 Prompt for AI Agents
In apps/server/src/utils/anchor.utils.ts around lines 101 to 103, the code
accesses error.logs without confirming that error is an object and that logs
exists. Add a check to ensure error is an object and error.logs is not null or
undefined before accessing error.logs to prevent runtime errors when error is a
primitive value.
Summary by CodeRabbit
New Features
Documentation
Refactor
Style
Chores