Conversation
reednaa
left a comment
There was a problem hiding this comment.
A few nits. Ideally we simplify the library rather than add to it.
reednaa
left a comment
There was a problem hiding this comment.
The added files does not have test coverage unlike all other files in this repository.
Provide coverage of solanaStandard.spec.ts.
Here are a variety of agent findings:
src/intent/solanaStandard.ts
- The amount field in mandateInputSchema uses "u64" but borshEncodeSolanaOrder passes BigInt(order.input.amount) — this is fine for
u64, but the amount in mandateOutputSchema uses bytes32 (big-endian 32-byte encoding), which is inconsistent. Verify this matches
the SVM program's actual layout. - The standardOrderToSolanaOrder conversion only uses the first input (order.inputs[0]), silently dropping all others. A
StandardOrder can have multiple inputs — should this throw if inputs.length > 1 rather than silently truncating? - borshEncodeSolanaOrder is exported but not re-exported from src/intent/index.ts. Intentional or oversight?
src/intent/helpers/output-encoding.ts
- outputSettler is now computed per-output via getSettler?.(token.chainId) ?? COIN_FILLER — this is a meaningful behavioral change
for all EVM orders too (previously settled globally). Ensure no regression for existing EVM paths.
src/helpers/convert.ts
- The early-return guard for already-bytes32 addresses is reasonable, but a Solana address (base58 string) would fail silently or
produce wrong output if passed here. The comment says "Accept only EVM addresses here" but there's no enforcement — could confuse
callers
src/helpers/convert.ts
Outdated
|
|
||
| export function addressToBytes32(address: `0x${string}`): `0x${string}` { | ||
| if (address.length === 66) return address; // already bytes32 with 0x prefix | ||
| if (address.length === 64) return `0x${address}`; // already bytes32 without 0x |
There was a problem hiding this comment.
Does this properly catch 64 length addresses prefixed with 0x?
I would rather use something like: 0x${address.replace("0x", "").padStart("0", 64)}
There was a problem hiding this comment.
yeah i think we should remove if (address.length === 64) also we don't need to check the address length in address.length !== 40 for EVM since we are already constraining the template with address: 0x${string} in the parameters.
| asOrder(): TOrder; | ||
| inputChains(): bigint[]; | ||
| orderId(): `0x${string}`; | ||
| compactClaimHash(): `0x${string}`; |
There was a problem hiding this comment.
This is required. Add a handler for EVM vs Solana and then provide compactClaimHash if chain is identified as EVM.
The PR accidentally swapped compact↔escrow settler assignments. Restores the original: compact/single→COMPACT_LIFI, compact/multi→MULTICHAIN_COMPACT, escrow/single→ESCROW_LIFI, escrow/multi→MULTICHAIN_ESCROW. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| export const SOLANA_MAINNET_CHAIN_ID = 1151111081099710n; | ||
| export const SOLANA_TESTNET_CHAIN_ID = 1151111081099711n; | ||
| export const SOLANA_DEVNET_CHAIN_ID = 1151111081099712n; |
There was a problem hiding this comment.
Do we want to consider renaming these to just SOLANA_MAINNET, SOLANA_TESTNET, etc?
src/types/order.ts
Outdated
| export type SolanaStandardOrder = { | ||
| user: `0x${string}`; | ||
| nonce: bigint; | ||
| originChainId: bigint; | ||
| expires: number; | ||
| fillDeadline: number; | ||
| inputOracle: `0x${string}`; | ||
| input: MandateInput; | ||
| outputs: MandateOutput[]; | ||
| }; |
There was a problem hiding this comment.
I really hate this type. It does not feel welcome. Why can we not use StandardOrder?
Then when we use it, we just cast it? The alternative is using a compatibility layer between EVM and Solana then moving that into StandardOrder?
export type StandardEVM = {
user: `0x${string}`;
nonce: bigint;
originChainId: bigint;
expires: number;
fillDeadline: number;
inputOracle: `0x${string}`;
inputs: [bigint, bigint][];
outputs: MandateOutput[];
};
export type StandardSolana = {
user: `0x${string}`;
nonce: bigint;
originChainId: bigint;
expires: number;
fillDeadline: number;
inputOracle: `0x${string}`;
inputs: [bigint, bigint][1];
outputs: MandateOutput[];
};
export type StandardIntent = StandardEVM | StandardSolana;
The only other alternative is to use an abstract representation of the intent for longer.
Summary
StandardSolanaorder type — a single-input variant ofStandardOrderfor Solana input settlersborshEncodeSolanaOrder) and order ID derivation (computeStandardSolanaId) matching thecommon::types::StandardOrderlayout incatalyst-intent-svmbuildMandateOutputs, falling back toCOIN_FILLERfor EVM outputsoutputRecipientoption toCreateIntentOptionsto support a different recipient than the connected walletCoreTokenwithchainNameSpace: "eip155" | "solana"(CAIP namespace) for output routingDesign notes
StandardSolanaenforces a single input — the Solana program only supports one SPL token input per order.standardOrderToSolanaOrderthrows if called with zero or more than one input.amountis encoded asu64(Solana stores SPL amounts as u64); outputamountisbytes32to match the EVM ABI layout.undefinedwith TODO comments —inputSettlerForSolanaandbuildMandateOutputsthrow for undeployed chains.Testing
src/intent/solanaStandard.spec.ts— full coverage: Borsh encoding golden value, u32 overflow guards, mutation tests, conversion helpers,StandardSolanaIntentclasssrc/intent/helpers/output-encoding.spec.ts— Solana PDA routing, unsupported chain rejection, EVM regression coveragesrc/intent/helpers/shared.spec.ts—inputSettlerForSolanahappy path and error pathsbun test