[LWDM] feat(coin-solana): [LIVE-27766] alpaca api validateIntent#15586
[LWDM] feat(coin-solana): [LIVE-27766] alpaca api validateIntent#15586cted-ledger merged 1 commit intodevelopfrom
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR implements the validateIntent Alpaca API method for Solana along with supporting utility functions for address validation, token conversion, sequence retrieval, and intent type mapping. The implementation provides comprehensive validation logic for both regular and staking transactions on Solana, including proper handling of native SOL and SPL token transfers.
Changes:
- Implements
validateIntentwith comprehensive validation logic for Solana transaction intents - Adds supporting utility functions:
validateAddress,getNextSequence,getTokenFromAsset,getAssetFromToken, andcomputeIntentType - Integrates new functions into the Alpaca and Bridge API factories
- Includes extensive unit, integration, and mocked tests for all new functionality
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| libs/coin-modules/coin-solana/src/logic/validateIntent.ts | Main validation logic for transaction intents, handling both regular and staking operations with error/warning detection |
| libs/coin-modules/coin-solana/src/logic/validateAddress.ts | Simple wrapper for Base58 address validation |
| libs/coin-modules/coin-solana/src/logic/getTokenFromAsset.ts | Token/asset conversion utilities for linking Ledger TokenCurrency with Alpaca AssetInfo |
| libs/coin-modules/coin-solana/src/logic/getNextSequence.ts | Retrieves current Solana slot for transaction sequencing |
| libs/coin-modules/coin-solana/src/logic/computeIntentType.ts | Maps transaction modes to granular intent types for staking operations |
| libs/coin-modules/coin-solana/src/api/index.ts | API factory refactored to separate AlpacaApi and BridgeApi, combining both in createApi |
| libs/coin-modules/coin-solana/src/api/index.unit.test.ts | Comprehensive test coverage for API factory with mocked logic functions |
| libs/coin-modules/coin-solana/src/logic/tests/ | Multiple test files with unit, integration, and mocked test coverage |
| .changeset/odd-mugs-complain.md | Changeset documenting the minor version bump |
Web Tools Build Status
|
1b5b66b to
233baa9
Compare
233baa9 to
afdddfa
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements the validateIntent Alpaca API method for Solana, along with supporting utilities for address validation, sequence numbering, token-asset conversion, and intent type computation. These additions extend the Solana coin module's API compliance with the Alpaca framework.
Changes:
- Implements
validateIntentfunction with comprehensive validation logic for native transfers, token transfers, and staking operations - Adds
validateAddressfunction for Base58 address validation - Introduces helper functions
getTokenFromAssetandgetAssetFromTokenfor bidirectional conversion between Alpaca and Ledger token representations - Adds
getNextSequenceandcomputeIntentTypefunctions to support the generic-alpaca bridge - Refactors API factory to split AlpacaApi and BridgeApi implementations with comprehensive test coverage
- Includes changeset documenting the new
validateIntentimplementation
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 |
|---|---|
| libs/coin-modules/coin-solana/src/logic/validateIntent.ts | Core validation logic for transaction intents with native transfer, token transfer, and staking support |
| libs/coin-modules/coin-solana/src/logic/validateAddress.ts | Base58 address validation wrapper |
| libs/coin-modules/coin-solana/src/logic/getTokenFromAsset.ts | Bidirectional token-asset conversion utilities |
| libs/coin-modules/coin-solana/src/logic/getNextSequence.ts | Sequence number fetching from RPC |
| libs/coin-modules/coin-solana/src/logic/computeIntentType.ts | Intent type mapping for generic-alpaca bridge |
| libs/coin-modules/coin-solana/src/api/index.ts | API factory refactoring to expose AlpacaApi and BridgeApi methods |
| libs/coin-modules/coin-solana/src/logic/tests/ | Comprehensive unit and integration tests for all new functions |
| .changeset/odd-mugs-complain.md | Changeset documentation |
| if ( | ||
| isStakingTransactionIntent(transactionIntent) || | ||
| transactionIntent.type === "stake.withdraw" | ||
| ) { |
There was a problem hiding this comment.
why checking transactionIntent.type === "stake.withdraw" ? tx.intentType is not "staking" when we do a withdraw ?
There was a problem hiding this comment.
stake.withdraw is a Solana-specific staking operation that doesn't fit the generic StakingTransactionIntent type from coin-framework, which requires mode: StakingOperation ("delegate" | "undelegate" | "redelegate") and valAddress. Since we can't extend coin-framework types here, stake.withdraw intents end up with
intentType: "transaction" by default, making isStakingTransactionIntent return false for them. The explicit || intent.type === "stake.withdraw" check is intentional to route these intents through validateStakingIntent, which correctly handles them (see the stake.withdraw case in the switch).
There was a problem hiding this comment.
I agree it feels something if off here. We can discuss adding modes in the coin framework meeting (I am afraid it delays the migration to alpaca-coin-module even more, but that is another story).
Also what about "stake.createAccount" ? I don't see this mode in StakingOperation.
afdddfa to
6a175d3
Compare
38738a7 to
24ec6ae
Compare
24ec6ae to
b31b22d
Compare
| warnings, | ||
| estimatedFees, | ||
| amount, | ||
| totalSpent: estimatedFees, |
There was a problem hiding this comment.
In validateStakingIntent, totalSpent is always set to estimatedFees. For stake.createAccount, the main account balance decreases by amount + estimatedFees (consistent with existing Live logic in src/getTransactionStatus.ts, where stake.createAccount totalSpent is amount + fee). Returning only the fees here will under-report totalSpent for the intent validation.
Consider computing totalSpent per staking intent type (e.g., stake.createAccount => amount + estimatedFees, while stake.delegate/stake.undelegate/stake.withdraw remain estimatedFees).
| totalSpent: estimatedFees, | |
| totalSpent: | |
| intent.type === "stake.createAccount" ? amount + estimatedFees : estimatedFees, |
|
|
||
| expect(result.errors).toEqual({}); | ||
| expect(result.amount).toBe(1_000_000_000n); | ||
| expect(result.totalSpent).toBe(5000n); |
There was a problem hiding this comment.
The expectation result.totalSpent equals only the fee (5000n) for stake.createAccount is likely incorrect if totalSpent represents the sender’s native balance decrease. stake.createAccount moves amount out of the main account (into the stake account), so totalSpent should include amount + fees (as coin-solana’s existing getTransactionStatus does for stake.createAccount).
| expect(result.totalSpent).toBe(5000n); | |
| expect(result.totalSpent).toBe(1_000_000_000n + 5000n); |
| if ( | ||
| isStakingTransactionIntent(transactionIntent) || | ||
| transactionIntent.type === "stake.withdraw" | ||
| ) { |
There was a problem hiding this comment.
I agree it feels something if off here. We can discuss adding modes in the coin framework meeting (I am afraid it delays the migration to alpaca-coin-module even more, but that is another story).
Also what about "stake.createAccount" ? I don't see this mode in StakingOperation.
| errors: Record<string, Error>, | ||
| ): bigint { | ||
| if (!intent.recipient) { | ||
| errors.recipient = new RecipientRequired(""); |
There was a problem hiding this comment.
| errors.recipient = new RecipientRequired(""); | |
| errors.recipient = new RecipientRequired(); |
| export async function getNextSequence(api: ChainAPI, _address: string): Promise<bigint> { | ||
| const slot = await api.connection.getSlot(); | ||
| return BigInt(slot); |
There was a problem hiding this comment.
getSlot is the latest block height, nothing to do with the transaction sequence number for a particular address.
This one is tricky though, because Solana does not use this concept of sequence number. I see we just compute it manually here and here.
We could make a little study
- if we don't care about differents numbers, what are the impact of returning 0 ?
- if we care about increasing differents numbers, what are the impact of returning the timestamp ?
I am saying this to avoid HTTP calls
There was a problem hiding this comment.
Solana doesn't have sequence numbers per address. getNextSequence was calling getSlot() only to produce a large enough number to avoid collisions with confirmed op sequences. Date.now() serves the same purpose without the RPC call.
here is the new implementation for getNextSequence:
export function getNextSequence(_address: string): bigint {
return BigInt(Date.now());
}| b.asset.type !== "native" && | ||
| "assetReference" in b.asset && | ||
| "assetReference" in (intent.asset ?? {}) && | ||
| (b.asset as { assetReference: string }).assetReference === | ||
| (intent.asset as { assetReference: string }).assetReference, |
There was a problem hiding this comment.
| b.asset.type !== "native" && | |
| "assetReference" in b.asset && | |
| "assetReference" in (intent.asset ?? {}) && | |
| (b.asset as { assetReference: string }).assetReference === | |
| (intent.asset as { assetReference: string }).assetReference, | |
| b.asset.type !== "native" && | |
| "assetReference" in b.asset && | |
| "assetReference" in intent.asset && | |
| b.asset.assetReference === intent.asset.assetReference, |
I see this pattern twice, you can consider having a helper.
You may be interested in the EVM equivalent that I believe quite convenient to use:
ledger-live/libs/coin-modules/coin-evm/src/logic/validateIntent.ts
Lines 41 to 53 in b750326
There was a problem hiding this comment.
pattern applied
| }; | ||
| } | ||
|
|
||
| export function computeIntentType(transaction: Record<string, unknown>): string { |
There was a problem hiding this comment.
Maybe we can make this method evolve to be something like
type IntentType =
| { intentType: 'transaction', type: string },
| { intentType: 'staking', mode: StakingOperation, type: string }
computeIntentType(transaction: Record<string, unknown>): IntentTypeNot in the scope of this PR though
b31b22d to
b1e60f2
Compare
b1e60f2 to
39d4880
Compare
|



✅ Checklist
npx changesetwas attached.📝 Description
Implements the
validateIntentAlpaca API method for Solana❓ Context
🧐 Checklist for the PR Reviewers