Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds Stellar (Soroban) support across frontend and backend: database schema (Token.assetIssuer), token/chain kind fields, signature handling for EVM and Stellar, Stellar wallet/provider and SEP-10 auth, Soroban invocation/trustline utilities, ABI and types updates, and multiple frontend/ backend DTO/service adaptations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Relayer
participant StellarRPC
participant Database
rect rgba(100,149,237,0.5)
User->>Frontend: open login (Stellar)
Frontend->>StellarRPC: request connect / sign XDR (StellarWalletProvider)
StellarRPC-->>Frontend: signed XDR
Frontend->>Relayer: POST /v1/auth/login (signed XDR, chainKind=STELLAR)
Relayer->>Relayer: verify SEP-10 sig (stellar.service)
Relayer->>Database: lookup/create user, issue tokens
Relayer-->>Frontend: return tokens (auth_token, refresh_token)
Frontend->>User: store cookies, reload
end
sequenceDiagram
participant User
participant Frontend
participant Relayer
participant SorobanRPC
participant EVM
rect rgba(34,139,34,0.5)
User->>Frontend: create/ad (SAC token)
Frontend->>Frontend: ensure trustline (trustline.ts) -> if missing sign+submit
Frontend->>SorobanRPC: invoke create_ad (invokeSoroban via StellarAdapter)
SorobanRPC-->>Frontend: txHash
Frontend->>Relayer: POST createAd (chainKind=STELLAR, reqContractDetails)
Relayer->>Database: persist ad with assetIssuer
Relayer-->>Frontend: ad created response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/frontend/package.json (1)
32-33:⚠️ Potential issue | 🟡 MinorReact and ReactDOM version mismatch.
reactis pinned to19.1.0whilereact-domis at19.2.5. These packages should typically use matching versions to avoid potential subtle runtime inconsistencies or compatibility issues.Proposed fix
- "react": "19.1.0", - "react-dom": "19.2.5", + "react": "19.2.5", + "react-dom": "19.2.5",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/package.json` around lines 32 - 33, The package.json has mismatched React versions: "react" is 19.1.0 while "react-dom" is 19.2.5; update the dependencies so both "react" and "react-dom" use the same version (pick a single canonical version, e.g., set "react" to "19.2.5" to match "react-dom" or align both to a chosen release), then reinstall to update the lockfile (npm/yarn/pnpm install) and run the app/tests/build to verify compatibility; modify the "react" and/or "react-dom" entries in package.json accordingly.apps/backend-relayer/src/modules/trades/trade.service.ts (1)
1291-1291:⚠️ Potential issue | 🟡 MinorRemove debug console.log statement.
This appears to be a debug artifact that should be removed before merging:
- console.log(dto); - return {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend-relayer/src/modules/trades/trade.service.ts` at line 1291, Remove the debug console.log(dto) statement found in trade.service.ts (inside the relevant TradeService method handling the DTO) — delete the console.log call or replace it with the module's proper logger (e.g., this.logger.debug or processLogger.debug) if the DTO needs to be recorded, ensuring no raw console logs remain in production code.
🧹 Nitpick comments (12)
apps/backend-relayer/src/modules/ads/dto/ad.dto.ts (1)
248-259: LGTM — consider tightening the TypeScript type for consistency.The addition of
chainKindand the expandedkindenum correctly support Stellar token types. Note thatTokenDto.chainKindis typed asstringwhile the response DTOs (e.g.,CreateAdResponseDto.chainKind) use the stricter'EVM' | 'STELLAR'union. For consistency, consider:- chainKind!: string; + chainKind!: 'EVM' | 'STELLAR';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend-relayer/src/modules/ads/dto/ad.dto.ts` around lines 248 - 259, TokenDto has loose string types for chainKind and kind; tighten them to match the response DTO unions by changing the property types on the TokenDto class: set chainKind to the literal union 'EVM' | 'STELLAR' and set kind to the literal union 'ERC20' | 'NATIVE' | 'SAC' | 'SEP41' (keeping the existing `@ApiProperty` enum metadata as-is) so consumers and CreateAdResponseDto remain consistent with these exact values.apps/backend-relayer/src/modules/routes/route.service.ts (1)
93-107: Extract duplicated token select into a shared constant.The same nested
adToken/orderTokenselect shape is repeated across list/get/create, which makes schema evolution error-prone.Refactor sketch
+const tokenSelect = { + id: true, + symbol: true, + name: true, + address: true, + decimals: true, + kind: true, + assetIssuer: true, + chain: { select: { id: true, name: true, chainId: true, kind: true } }, +} as const; // then reuse: adToken: { - select: { ...full object... } + select: tokenSelect }, orderToken: { - select: { ...full object... } + select: tokenSelect },Also applies to: 155-168, 239-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend-relayer/src/modules/routes/route.service.ts` around lines 93 - 107, Extract the repeated nested select shape for adToken/orderToken into a shared constant (e.g., TOKEN_SELECT or TOKEN_WITH_CHAIN) in route.service.ts and replace the duplicated select objects used in the list/get/create query calls; specifically locate the repeated select blocks referenced for adToken and orderToken in the functions that perform list/get/create operations and substitute them with the single shared constant (export it if other modules need it), ensuring the constant includes the chain: { select: { id, name, chainId, kind } } structure and the token fields id, symbol, name, address, decimals, kind, assetIssuer so all queries reference the same shape.apps/backend-relayer/prisma/schema.prisma (1)
86-88: EnforceassetIssuer/Token.kindinvariant at the database layer.Right now this rule exists only in comments. Add a DB check constraint so data can’t drift from the intended SAC-only semantics.
Proposed migration snippet
+-- In a new SQL migration: +ALTER TABLE "Token" +ADD CONSTRAINT "token_assetIssuer_kind_check" +CHECK ( + ("kind" = 'SAC' AND "assetIssuer" IS NOT NULL) + OR + ("kind" <> 'SAC' AND "assetIssuer" IS NULL) +);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend-relayer/prisma/schema.prisma` around lines 86 - 88, Add a DB-level CHECK constraint so Token.kind and assetIssuer cannot drift from the SAC-only rule: ensure that when Token.kind = 'SAC' then assetIssuer IS NOT NULL, and when Token.kind != 'SAC' then assetIssuer IS NULL. Implement this by adding a SQL migration that runs ALTER TABLE "Token" ADD CONSTRAINT token_assetissuer_kind_check CHECK ((kind = 'SAC' AND assetIssuer IS NOT NULL) OR (kind <> 'SAC' AND assetIssuer IS NULL)); and update any Prisma migration metadata so the constraint is tracked (keep the Prisma schema Token model unchanged except for mapping if needed).apps/frontend/lib/chains.ts (1)
9-17: Consider externalizing the Stellar chain ID instead of hard-coding it.If backend seed/config changes, this can silently hide valid chains in selectors. Prefer env/config-driven wiring.
Possible refactor
-export const STELLAR_TESTNET_CHAIN_ID = "1000001" +export const STELLAR_TESTNET_CHAIN_ID = + process.env.NEXT_PUBLIC_STELLAR_TESTNET_CHAIN_ID ?? "1000001"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/lib/chains.ts` around lines 9 - 17, Replace the hard-coded STELLAR_TESTNET_CHAIN_ID with a config-driven value so UI selectors reflect backend changes: read the chain id from a runtime config or env (e.g. NEXT_PUBLIC_STELLAR_TESTNET_CHAIN_ID) and fall back to the existing "1000001" constant; update the STELLAR_TESTNET_CHAIN_ID declaration and ensure VISIBLE_CHAIN_IDS uses that config value (referencing STELLAR_TESTNET_CHAIN_ID and VISIBLE_CHAIN_IDS) so changes to backend seed/config automatically surface in the front-end selectors.apps/frontend/components/ad-management-ui/AddLiquidity.tsx (1)
109-109: Silent error swallowing in handleCreateAd.The empty catch block discards all errors, making debugging difficult. Consider at least logging the error or showing a toast notification.
♻️ Proposed fix
- } catch (error) { } + } catch (error) { + console.error("Failed to create ad:", error) + // Consider adding a toast notification here + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/components/ad-management-ui/AddLiquidity.tsx` at line 109, The catch block in handleCreateAd silently swallows errors; modify the catch in AddLiquidity's handleCreateAd to log the error (e.g., console.error or process logger) and/or surface a user-facing notification (toast) with a helpful message; ensure you reference the caught error variable and include context like "Failed to create ad" when logging or notifying so debugging and UX are improved.apps/frontend/components/bridge-ui/BridgeTab.tsx (1)
48-52: Inconsistent API shape in commented code.The commented code checks
chains?.data.lengthbut accesseschains.rows[0]. If this code is uncommented, it will fail. Consider either fixing or removing this dead code.♻️ Fix or remove
- // useEffect(() => { - // if (chains?.data.length) { - // setSelectedBaseChainId(chains.rows[0].chainId) - // } - // }, [chains]) + // Removed stale commented code that referenced old API shape🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/components/bridge-ui/BridgeTab.tsx` around lines 48 - 52, The commented useEffect contains an inconsistent API check: it checks chains?.data.length but then accesses chains.rows[0].chainId; update the code so the shape is consistent (either check chains?.rows?.length before using chains.rows[0].chainId, or use chains?.data[0].chainId and setSelectedBaseChainId accordingly), or if this block is dead, remove the commented code entirely; locate the useEffect block and fix the condition and access to match the actual API shape used elsewhere in the component (symbols: useEffect, chains, rows, data, setSelectedBaseChainId).apps/frontend/lib/stellar-adapter.ts (1)
6-7: Consider warning when falling back to testnet defaults in production.The silent fallback to testnet URLs when
NEXT_PUBLIC_STELLAR_*env vars are unset could cause production issues. Consider logging a warning or validating env vars at startup.♻️ Optional: Add development-time warning
+const warnMissingEnv = (name: string, fallback: string) => { + if (process.env.NODE_ENV === "production" && !process.env[name]) { + console.warn(`Missing ${name}, falling back to ${fallback}`) + } +} + export function useStellarAdapter(): { ... const buildCtx = (): StellarAdapterCtx => { ... const rpcUrl = process.env.NEXT_PUBLIC_STELLAR_RPC_URL || DEFAULT_TESTNET_RPC + warnMissingEnv("NEXT_PUBLIC_STELLAR_RPC_URL", DEFAULT_TESTNET_RPC)Also applies to: 20-21, 34-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/lib/stellar-adapter.ts` around lines 6 - 7, When building the Stellar adapter, detect when NEXT_PUBLIC_STELLAR_RPC and NEXT_PUBLIC_STELLAR_HORIZON are unset and you're using the fallback constants DEFAULT_TESTNET_RPC or DEFAULT_TESTNET_HORIZON; in that case emit a clear warning (or throw/configure a startup validation failure) so production deployments don’t silently use testnet endpoints. Update the initialization logic in apps/frontend/lib/stellar-adapter.ts to check process.env.NEXT_PUBLIC_STELLAR_RPC and process.env.NEXT_PUBLIC_STELLAR_HORIZON, and if running in production (NODE_ENV === "production") log a warning/error that the code is falling back to DEFAULT_TESTNET_RPC/DEFAULT_TESTNET_HORIZON and suggest setting the env vars.apps/frontend/hooks/useStellarAuth.ts (1)
41-44: Consider adding cookie security attributes.The auth tokens are stored without
SecureorSameSiteattributes. WhileHttpOnlycannot be set client-side, addingSecureandSameSiteimproves security posture:- Cookies.set("auth_token", data.tokens.access) - Cookies.set("refresh_token", data.tokens.refresh) + Cookies.set("auth_token", data.tokens.access, { + secure: true, + sameSite: "strict", + }) + Cookies.set("refresh_token", data.tokens.refresh, { + secure: true, + sameSite: "strict", + })Also note that
return dataon line 44 is unreachable afterwindow.location.reload().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/hooks/useStellarAuth.ts` around lines 41 - 44, The code in useStellarAuth.ts sets auth cookies via Cookies.set("auth_token", ...) and Cookies.set("refresh_token", ...) without security attributes and then calls window.location.reload(), making the subsequent return data unreachable; update the Cookies.set calls to include Secure: true and SameSite: 'Lax' or 'Strict' (and consider a sensible expires if needed) so tokens are set with those attributes, and either remove the unreachable return data or move the return before window.location.reload() (or replace reload with navigation that preserves the return flow) to ensure reachable control flow.apps/frontend/hooks/useAds.ts (1)
378-402: Consider passing ad token info to avoid redundant fetch.Both
useWithdrawFundsanduseCloseAdcallgetSingleAd(data.adId)solely to retrieveadTokenfor the trustline check. If the caller already has this information (e.g., from the ad detail page context), consider extendingIWithdrawFromAdRequest/ICloseAdRequestto optionally includeadToken, avoiding the extra network round-trip:// In the request type adToken?: IAdToken; // Optional: pass if available to skip fetch // In the mutation const adToken = data.adToken ?? (await getSingleAd(data.adId)).adToken; await assertRecipientTrustline(adToken, data.to);This is a minor optimization and not blocking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/hooks/useAds.ts` around lines 378 - 402, Add an optional adToken field to the request types (IWithdrawFromAdRequest and ICloseAdRequest) and update the withdrawal/close flows to use it when present: in the code path that currently calls getSingleAd(data.adId) (see getSingleAd and assertRecipientTrustline usage in useWithdrawFunds/useCloseAd handlers), resolve adToken as const adToken = data.adToken ?? (await getSingleAd(data.adId)).adToken and pass that to assertRecipientTrustline(adToken, data.to) to avoid the redundant fetch when the caller already supplies the token.apps/frontend/utils/stellar/invoke.ts (1)
40-40: Default to TESTNET could be risky in production.The fallback to
Networks.TESTNETwhennetworkPassphraseis undefined could cause issues if a caller forgets to pass the passphrase in a production environment. Consider makingnetworkPassphraserequired or logging a warning when using the default:- const passphrase = networkPassphrase ?? Networks.TESTNET + if (!networkPassphrase) { + console.warn("invokeSoroban: networkPassphrase not provided, defaulting to TESTNET") + } + const passphrase = networkPassphrase ?? Networks.TESTNETThis helps catch configuration mistakes during development.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/utils/stellar/invoke.ts` at line 40, The code currently falls back to Networks.TESTNET via the variable passphrase (const passphrase = networkPassphrase ?? Networks.TESTNET), which is risky; update invoke.ts so networkPassphrase is required or, if keeping optional, emit a clear warning/error when undefined and before defaulting to Networks.TESTNET; locate the related symbol networkPassphrase and the passphrase assignment in invoke.ts (or the containing function) and either change the parameter to be non-optional or add a runtime check that logs/processLogger.warn (or throws) when networkPassphrase is missing, then only use Networks.TESTNET as an explicit, clearly logged fallback.apps/frontend/hooks/useTrades.ts (1)
456-492: Consider extracting the orderParams transformation to reduce inline IIFE complexity.The inline IIFEs work correctly but add cognitive overhead. A helper function could improve readability.
♻️ Optional: Extract transformation helper
function toBigIntOrderParams<T extends { amount: string; salt: string }>( p: T, chainIdKey: 'adChainId' | 'orderChainId' ): Omit<T, 'amount' | 'salt' | typeof chainIdKey> & { amount: bigint; salt: bigint } & Record<typeof chainIdKey, bigint> { return { ...p, amount: BigInt(p.amount), [chainIdKey]: BigInt((p as any)[chainIdKey]), salt: BigInt(p.salt), } as any; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/hooks/useTrades.ts` around lines 456 - 492, The inline IIFEs converting orderParams to BigInt values create cognitive overhead; extract that logic into a helper (e.g., toBigIntOrderParams) and use it where the IIFEs are called inside the writeContractAsync args. Implement a generic helper that accepts the param object and the chainId key name (adChainId or orderChainId), converts amount, salt and the appropriate chain id to BigInt while preserving other fields (handle types IOrderPortalOrderParams and IAdManagerOrderParams), then replace the two IIFE usages in useTrades.ts with calls to this helper for clearer, reusable code.apps/frontend/components/providers/StellarWallet.tsx (1)
158-178: Consider guarding signing methods when wallet is not connected.The signing methods pass
address ?? undefinedto the SDK. If these are called before connection, the SDK behavior may be unpredictable. The UI likely guards against this, but a defensive check could improve robustness.💡 Optional defensive check
const signTransaction = useCallback( async (xdr: string, networkPassphrase?: string) => { + if (!address) throw new Error("Stellar wallet not connected") const { signedTxXdr } = await StellarWalletsKit.signTransaction(xdr, { networkPassphrase: networkPassphrase ?? Networks.TESTNET, - address: address ?? undefined, + address, }) return signedTxXdr }, [address], )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/components/providers/StellarWallet.tsx` around lines 158 - 178, signTransaction and signMessage call StellarWalletsKit even when address may be undefined, which can lead to unpredictable SDK behavior; add a defensive guard in both functions (signTransaction and signMessage) to check that address (or a wallet-connected flag) is present and throw or return a clear error if not, instead of calling StellarWalletsKit.signTransaction/signMessage with address ?? undefined; keep networkPassphrase logic as-is (Networks.TESTNET fallback) but ensure the functions early-exit when no connected address is available.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend-relayer/src/chain-adapters/adapters/evm-chain-adapter.ts`:
- Around line 186-193: The code casts signature to `0x${string}` without runtime
checking before calling `this.viem.verifyOrderSignature`; add a validation step
(similar to `assertLocalAddress`) to verify the signature is a hex string of the
expected length (e.g. matches /^0x[0-9a-fA-F]{130}$/) before casting. Implement
or reuse a helper like `assertHexSignature(signature, 'signature')` and call it
in the same function (the one that calls `this.viem.verifyOrderSignature`) so
the unsafe cast to `signature as \`0x\${string}\`` is only done after the check.
Ensure the validation throws the same style of error as `assertLocalAddress` on
failure.
In `@apps/backend-relayer/src/modules/token/dto/token.dto.ts`:
- Around line 131-138: The assetIssuer field is only being string-validated but
needs format and conditional requirement when token kind is SAC; update the DTO
to replace the loose `@IsString`()/@IsOptional() with a conditional validator like
`@ValidateIf`(o => o.kind === 'SAC') plus `@IsNotEmpty`() and a strict Stellar
public-key pattern check (e.g. Matches(/^G[A-Z2-7]{55}$/) or an equivalent
StrKey validator) to ensure correct G-strkey format; also apply the same change
to the other occurrence of assetIssuer in the file (the block around lines
~201-208) so both DTOs enforce format and require the issuer when kind ===
'SAC'.
In `@apps/backend-relayer/src/modules/token/token.service.ts`:
- Around line 218-226: The current check throws whenever dto.kind !== 'SAC'
regardless of the previous value; update the condition to only enforce clearing
assetIssuer when the existing record is SAC and the new kind moves away from SAC
(i.e., change the if to verify exists.kind === 'SAC' && dto.kind !== 'SAC'). In
the block referencing dto.kind and exists.kind (in token.service.ts around the
kind-change logic), keep the same error message and behavior but only trigger it
for transitions originating from 'SAC'.
In `@apps/frontend/components/bridge-ui/TradeAd.tsx`:
- Around line 92-104: The conditional block uses optional chaining with non-null
assertions like balance?.data?.value! and nativeBalance?.data?.decimals! which
is contradictory and unsafe; since you already check balance.data and
nativeBalance.data in the if/else guards, replace the optional-chained
expressions with direct property access (e.g., balance.data.value and
balance.data.decimals, nativeBalance.data.value and nativeBalance.data.decimals)
when calling formatUnits and keep setBalance_value as-is; update the references
in the effect that depends on balance.data, nativeBalance.data,
stellarBalance.data, isStellarOrder to remove the unnecessary ?. and !.
In `@apps/frontend/hooks/useTrades.ts`:
- Around line 347-386: The EIP-712 struct types for cross-chain address fields
in the signTypedDataAsync call are incorrect (currently "address") and must be
changed to "bytes32" to match the contract; update the Order type declaration in
signTypedDataAsync to use bytes32 for orderChainToken, adChainToken, bridger,
orderPortal, orderRecipient, adManager, adCreator, and adRecipient, and
transform the corresponding values pulled from params into 32-byte padded
representations (e.g., pad EVM addresses as bytes32(uint256(uint160(address)))
or an equivalent JS utility that left-pads/truncates to 32 bytes) while keeping
numeric fields (amount, orderChainId, adChainId, salt) as uint256/BigInt and
leaving other fields (adId) as their correct types so the produced EIP-712
message matches the contract ORDER_TYPEHASH.
In `@apps/frontend/package.json`:
- Around line 13-17: Update the dependency entry for `@rainbow-me/rainbowkit` in
package.json from "^2.2.8" to "^2.2.10"; locate the "@rainbow-me/rainbowkit"
entry and bump the version string to "^2.2.10", then run your package manager
(npm/yarn/pnpm install) to update lockfiles and verify the app builds/tests
pass.
In `@apps/frontend/services/chains.service.ts`:
- Line 14: The returned response type currently forces nextCursor to be a
non-null string; update the cast at the return in chains.service.ts so the
response is typed as { data: IChain[]; nextCursor: string | null } (i.e., make
nextCursor nullable) to avoid propagating unsafe non-null assumptions in
pagination logic where nextCursor may be absent. Ensure any callers of the
function handle nextCursor possibly being null.
In `@apps/frontend/types/ads.ts`:
- Around line 28-29: The chainId type is inconsistent across response
interfaces—ICreateAdResponse.chainId is number while IFundAdResponse.chainId,
IWithdrawFromAdResponse.chainId, and ICloseAdResponse.chainId are string; change
ICreateAdResponse.chainId to string so all four response interfaces use the same
type (chainId: string), and then update any local usages/call sites that assume
a number to use string or parse as needed; target the interfaces named
ICreateAdResponse, IFundAdResponse, IWithdrawFromAdResponse, and
ICloseAdResponse and the chainId property when making the change.
In `@apps/frontend/types/chains.ts`:
- Around line 3-10: The IChain interface should be converted to a discriminated
union keyed by ChainKind so EVM chains keep adManagerAddress/orderPortalAddress
typed as viem's Address while STELLAR chains use a plain string (hex/strkey)
type; update the type definition for ChainKind/IChain to two variants (e.g., {
kind: "EVM"; adManagerAddress: Address; orderPortalAddress: Address } | { kind:
"STELLAR"; adManagerAddress: string; orderPortalAddress: string }) and then
update callers (notably functions in trade.service.ts that currently cast
stellar values with `as \`0x${string}\``) to narrow by chain.kind before using
the address so the casts are unnecessary and Stellar addresses can be decoded to
strkey at the RPC boundary (follow
apps/backend-relayer/src/providers/stellar/utils/address.ts decoding logic).
In `@apps/frontend/types/tokens.ts`:
- Around line 1-4: The Token address type currently uses viem's Address for all
TokenKind values; change this to a discriminated union so EVM kinds use Address
and Stellar kinds use a hex/string type: introduce a TokenAddress union (or
inline union on IToken) discriminated by kind (e.g., { kind: "NATIVE" | "ERC20";
address: Address } | { kind: "SAC" | "SEP41"; address: string /* or
`0x${string}` */ }), update IToken to reference that union instead of Address,
and update any other usages mentioned (routes.ts and ads.ts) to use the new
discriminated shape and corresponding conversion helpers.
In `@apps/frontend/utils/stellar/address.ts`:
- Around line 6-15: hex32ToBuffer currently accepts both 32-byte and 20-byte hex
(padding the latter), which can produce an invalid Ed25519 public key when
passed to hex32ToAccountId/StrKey.encodeEd25519PublicKey; update
hex32ToAccountId to validate that the input is a true 32-byte hex (reject
20-byte EVM-style addresses) or introduce a separate hex32OnlyToBuffer that only
accepts 32-byte hex and use that inside hex32ToAccountId; ensure the function(s)
throw a clear error for 20-byte inputs and reference hex32ToBuffer,
hex32ToAccountId (and StrKey.encodeEd25519PublicKey) so callers know to use the
32-byte-only path for StrKey operations.
---
Outside diff comments:
In `@apps/backend-relayer/src/modules/trades/trade.service.ts`:
- Line 1291: Remove the debug console.log(dto) statement found in
trade.service.ts (inside the relevant TradeService method handling the DTO) —
delete the console.log call or replace it with the module's proper logger (e.g.,
this.logger.debug or processLogger.debug) if the DTO needs to be recorded,
ensuring no raw console logs remain in production code.
In `@apps/frontend/package.json`:
- Around line 32-33: The package.json has mismatched React versions: "react" is
19.1.0 while "react-dom" is 19.2.5; update the dependencies so both "react" and
"react-dom" use the same version (pick a single canonical version, e.g., set
"react" to "19.2.5" to match "react-dom" or align both to a chosen release),
then reinstall to update the lockfile (npm/yarn/pnpm install) and run the
app/tests/build to verify compatibility; modify the "react" and/or "react-dom"
entries in package.json accordingly.
---
Nitpick comments:
In `@apps/backend-relayer/prisma/schema.prisma`:
- Around line 86-88: Add a DB-level CHECK constraint so Token.kind and
assetIssuer cannot drift from the SAC-only rule: ensure that when Token.kind =
'SAC' then assetIssuer IS NOT NULL, and when Token.kind != 'SAC' then
assetIssuer IS NULL. Implement this by adding a SQL migration that runs ALTER
TABLE "Token" ADD CONSTRAINT token_assetissuer_kind_check CHECK ((kind = 'SAC'
AND assetIssuer IS NOT NULL) OR (kind <> 'SAC' AND assetIssuer IS NULL)); and
update any Prisma migration metadata so the constraint is tracked (keep the
Prisma schema Token model unchanged except for mapping if needed).
In `@apps/backend-relayer/src/modules/ads/dto/ad.dto.ts`:
- Around line 248-259: TokenDto has loose string types for chainKind and kind;
tighten them to match the response DTO unions by changing the property types on
the TokenDto class: set chainKind to the literal union 'EVM' | 'STELLAR' and set
kind to the literal union 'ERC20' | 'NATIVE' | 'SAC' | 'SEP41' (keeping the
existing `@ApiProperty` enum metadata as-is) so consumers and CreateAdResponseDto
remain consistent with these exact values.
In `@apps/backend-relayer/src/modules/routes/route.service.ts`:
- Around line 93-107: Extract the repeated nested select shape for
adToken/orderToken into a shared constant (e.g., TOKEN_SELECT or
TOKEN_WITH_CHAIN) in route.service.ts and replace the duplicated select objects
used in the list/get/create query calls; specifically locate the repeated select
blocks referenced for adToken and orderToken in the functions that perform
list/get/create operations and substitute them with the single shared constant
(export it if other modules need it), ensuring the constant includes the chain:
{ select: { id, name, chainId, kind } } structure and the token fields id,
symbol, name, address, decimals, kind, assetIssuer so all queries reference the
same shape.
In `@apps/frontend/components/ad-management-ui/AddLiquidity.tsx`:
- Line 109: The catch block in handleCreateAd silently swallows errors; modify
the catch in AddLiquidity's handleCreateAd to log the error (e.g., console.error
or process logger) and/or surface a user-facing notification (toast) with a
helpful message; ensure you reference the caught error variable and include
context like "Failed to create ad" when logging or notifying so debugging and UX
are improved.
In `@apps/frontend/components/bridge-ui/BridgeTab.tsx`:
- Around line 48-52: The commented useEffect contains an inconsistent API check:
it checks chains?.data.length but then accesses chains.rows[0].chainId; update
the code so the shape is consistent (either check chains?.rows?.length before
using chains.rows[0].chainId, or use chains?.data[0].chainId and
setSelectedBaseChainId accordingly), or if this block is dead, remove the
commented code entirely; locate the useEffect block and fix the condition and
access to match the actual API shape used elsewhere in the component (symbols:
useEffect, chains, rows, data, setSelectedBaseChainId).
In `@apps/frontend/components/providers/StellarWallet.tsx`:
- Around line 158-178: signTransaction and signMessage call StellarWalletsKit
even when address may be undefined, which can lead to unpredictable SDK
behavior; add a defensive guard in both functions (signTransaction and
signMessage) to check that address (or a wallet-connected flag) is present and
throw or return a clear error if not, instead of calling
StellarWalletsKit.signTransaction/signMessage with address ?? undefined; keep
networkPassphrase logic as-is (Networks.TESTNET fallback) but ensure the
functions early-exit when no connected address is available.
In `@apps/frontend/hooks/useAds.ts`:
- Around line 378-402: Add an optional adToken field to the request types
(IWithdrawFromAdRequest and ICloseAdRequest) and update the withdrawal/close
flows to use it when present: in the code path that currently calls
getSingleAd(data.adId) (see getSingleAd and assertRecipientTrustline usage in
useWithdrawFunds/useCloseAd handlers), resolve adToken as const adToken =
data.adToken ?? (await getSingleAd(data.adId)).adToken and pass that to
assertRecipientTrustline(adToken, data.to) to avoid the redundant fetch when the
caller already supplies the token.
In `@apps/frontend/hooks/useStellarAuth.ts`:
- Around line 41-44: The code in useStellarAuth.ts sets auth cookies via
Cookies.set("auth_token", ...) and Cookies.set("refresh_token", ...) without
security attributes and then calls window.location.reload(), making the
subsequent return data unreachable; update the Cookies.set calls to include
Secure: true and SameSite: 'Lax' or 'Strict' (and consider a sensible expires if
needed) so tokens are set with those attributes, and either remove the
unreachable return data or move the return before window.location.reload() (or
replace reload with navigation that preserves the return flow) to ensure
reachable control flow.
In `@apps/frontend/hooks/useTrades.ts`:
- Around line 456-492: The inline IIFEs converting orderParams to BigInt values
create cognitive overhead; extract that logic into a helper (e.g.,
toBigIntOrderParams) and use it where the IIFEs are called inside the
writeContractAsync args. Implement a generic helper that accepts the param
object and the chainId key name (adChainId or orderChainId), converts amount,
salt and the appropriate chain id to BigInt while preserving other fields
(handle types IOrderPortalOrderParams and IAdManagerOrderParams), then replace
the two IIFE usages in useTrades.ts with calls to this helper for clearer,
reusable code.
In `@apps/frontend/lib/chains.ts`:
- Around line 9-17: Replace the hard-coded STELLAR_TESTNET_CHAIN_ID with a
config-driven value so UI selectors reflect backend changes: read the chain id
from a runtime config or env (e.g. NEXT_PUBLIC_STELLAR_TESTNET_CHAIN_ID) and
fall back to the existing "1000001" constant; update the
STELLAR_TESTNET_CHAIN_ID declaration and ensure VISIBLE_CHAIN_IDS uses that
config value (referencing STELLAR_TESTNET_CHAIN_ID and VISIBLE_CHAIN_IDS) so
changes to backend seed/config automatically surface in the front-end selectors.
In `@apps/frontend/lib/stellar-adapter.ts`:
- Around line 6-7: When building the Stellar adapter, detect when
NEXT_PUBLIC_STELLAR_RPC and NEXT_PUBLIC_STELLAR_HORIZON are unset and you're
using the fallback constants DEFAULT_TESTNET_RPC or DEFAULT_TESTNET_HORIZON; in
that case emit a clear warning (or throw/configure a startup validation failure)
so production deployments don’t silently use testnet endpoints. Update the
initialization logic in apps/frontend/lib/stellar-adapter.ts to check
process.env.NEXT_PUBLIC_STELLAR_RPC and process.env.NEXT_PUBLIC_STELLAR_HORIZON,
and if running in production (NODE_ENV === "production") log a warning/error
that the code is falling back to DEFAULT_TESTNET_RPC/DEFAULT_TESTNET_HORIZON and
suggest setting the env vars.
In `@apps/frontend/utils/stellar/invoke.ts`:
- Line 40: The code currently falls back to Networks.TESTNET via the variable
passphrase (const passphrase = networkPassphrase ?? Networks.TESTNET), which is
risky; update invoke.ts so networkPassphrase is required or, if keeping
optional, emit a clear warning/error when undefined and before defaulting to
Networks.TESTNET; locate the related symbol networkPassphrase and the passphrase
assignment in invoke.ts (or the containing function) and either change the
parameter to be non-optional or add a runtime check that logs/processLogger.warn
(or throws) when networkPassphrase is missing, then only use Networks.TESTNET as
an explicit, clearly logged fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1a6d7531-7c8c-4e4a-8769-135331154f74
⛔ Files ignored due to path filters (3)
apps/frontend/public/assets/logos/stellar-logo.svgis excluded by!**/*.svgapps/frontend/public/assets/logos/stellar-xlm-logo.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (66)
apps/backend-relayer/prisma/migrations/20260413000000_add_token_asset_issuer/migration.sqlapps/backend-relayer/prisma/schema.prismaapps/backend-relayer/src/chain-adapters/adapters/chain-adapter.abstract.tsapps/backend-relayer/src/chain-adapters/adapters/evm-chain-adapter.tsapps/backend-relayer/src/chain-adapters/adapters/stellar-chain-adapter.tsapps/backend-relayer/src/main.tsapps/backend-relayer/src/modules/admin/admin.controller.spec.tsapps/backend-relayer/src/modules/ads/ad.service.tsapps/backend-relayer/src/modules/ads/dto/ad.dto.tsapps/backend-relayer/src/modules/chains/chain.controller.spec.tsapps/backend-relayer/src/modules/chains/chain.service.tsapps/backend-relayer/src/modules/chains/dto/chain.dto.tsapps/backend-relayer/src/modules/routes/route.service.tsapps/backend-relayer/src/modules/token/dto/token.dto.tsapps/backend-relayer/src/modules/token/token.service.tsapps/backend-relayer/src/modules/trades/dto/trade.dto.tsapps/backend-relayer/src/modules/trades/trade.service.tsapps/backend-relayer/src/providers/stellar/stellar.service.tsapps/backend-relayer/src/types/index.d.tsapps/backend-relayer/test/e2e/chain.e2e-spec.tsapps/backend-relayer/test/e2e/token.e2e-spec.tsapps/frontend/abis/AdManager.abi.tsapps/frontend/abis/ERC20.abi.tsapps/frontend/abis/orderPortal.abi.tsapps/frontend/app/(app)/(auth-protected)/faucet/page.tsxapps/frontend/app/(app)/bridge/page.tsxapps/frontend/app/layout.tsxapps/frontend/components/ad-management-ui/AddLiquidity.tsxapps/frontend/components/bridge-ui/BridgeTab.tsxapps/frontend/components/bridge-ui/SellAdd.tsxapps/frontend/components/bridge-ui/TradeAd.tsxapps/frontend/components/connect-wallet/ConnectWalletButton.tsxapps/frontend/components/landing/Approach.tsxapps/frontend/components/landing/Features.tsxapps/frontend/components/landing/Hero-Alt.tsxapps/frontend/components/landing/Hero.tsxapps/frontend/components/landing/ReImagining.tsxapps/frontend/components/orders/OrdersTable.tsxapps/frontend/components/providers/RainbowKit.tsxapps/frontend/components/providers/StellarWallet.tsxapps/frontend/hooks/useAds.tsapps/frontend/hooks/useStellarAuth.tsapps/frontend/hooks/useTrades.tsapps/frontend/lib/chain-icons.tsapps/frontend/lib/chains.tsapps/frontend/lib/stellar-adapter.tsapps/frontend/package.jsonapps/frontend/services/ads.service.tsapps/frontend/services/auth.service.tsapps/frontend/services/chains.service.tsapps/frontend/types/ads.tsapps/frontend/types/chains.tsapps/frontend/types/tokens.tsapps/frontend/types/trades.tsapps/frontend/utils/amount.tsapps/frontend/utils/evm/address.tsapps/frontend/utils/evm/index.tsapps/frontend/utils/evm/typedData.tsapps/frontend/utils/format-address.tsapps/frontend/utils/stellar/actions.tsapps/frontend/utils/stellar/address.tsapps/frontend/utils/stellar/balance.tsapps/frontend/utils/stellar/index.tsapps/frontend/utils/stellar/invoke.tsapps/frontend/utils/stellar/scval.tsapps/frontend/utils/stellar/trustline.ts
💤 Files with no reviewable changes (1)
- apps/frontend/components/bridge-ui/SellAdd.tsx
Critical:
- useTrades: EIP-712 Order struct signs bytes32 for cross-chain address
fields to match on-chain ORDER_TYPEHASH; previously signed as `address`,
which produced a non-matching typeHash and broke signature verification.
- token.service: SAC-transition guard now checks `exists.kind === 'SAC'`
so NATIVE → SEP41 (neither SAC) no longer demands clearing assetIssuer.
Major:
- evm-chain-adapter: add assertLocalSignature hex-regex guard before
casting to `0x${string}` in verifyOrderSignature.
- token.dto: validate assetIssuer format (`/^G[A-Z2-7]{55}$/`) on create;
update accepts empty string as explicit clear sentinel.
- types/chains: IChain becomes a discriminated union (IEvmChain |
IStellarChain) so Stellar addresses aren't forced through viem Address.
Minor:
- TradeAd: drop redundant optional-chain+non-null-assert after the
balance.data guard.
- types/ads: ICreateAdResponse.chainId number → string to match siblings.
- chains.service: nextCursor typed as string | null.
- types/tokens: IToken split into IEvmToken | IStellarToken union.
- utils/stellar/address: hex32ToAccountId / hex32ToContractId now reject
20-byte EVM input via new hex32OnlyToBuffer helper to prevent bogus
StrKey output from zero-padded EVM addresses.
- package.json: bump @rainbow-me/rainbowkit ^2.2.8 → ^2.2.10.\
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stellar integrations
Adds end-to-end Stellar support to ProofBridge alongside the existing EVM flow: Soroban contracts, chain-kind-aware backend relayer, SEP-43 signing in the frontend, and full EVM↔Stellar cross-chain e2e tooling.
Scope
Backend relayer
ValidationPipe({ whitelist, forbidNonWhitelisted })globally; list endpoints standardized on{ data, nextCursor }(wasrows);IFundAdResponse.amount/IAd.minAmount|maxAmount/IUpdateAdResponseDTO types aligned with runtime shape.adToken/orderTokenembeds now includeassetIssuerfor client-side trustline checks.Frontend
StellarWalletProvider) built on@creit.tech/stellar-wallets-kitwith Freighter/Albedo/Lobstr/xBull/Rabet/Hana modules and asignMessageAPI for SEP-43 off-chain signatures.chainKindand picks the appropriate wallet (0xEVM vsG…strkey) for recipient addresses.verifyOrderSignaturedoes off-chain Ed25519 verification against"Stellar Signed Message:\n" + orderHash; no on-chain impact.isAdCreatorvia shape discrimination onorderParams(adManagerfield ⇒ OrderPortal side,srcOrderPortal⇒ AdManager side) — fixes a prior bug where address comparison broke under bytes32 padding on EVM.assertRecipientTrustlinepreflights withdraw/close to Stellar SAC recipients and surfaces a clear error when the destination has no trustline, instead of letting the Soroban call revert.TradeAdfor NATIVE/SAC tokens when the order chain is Stellar; wagmiuseBalanceis gated off on that branch.connectionActioninTradeAd— replaces nested ternaries and a dead "Connect Stellar wallet" button with a priority ladder: pay-side wallet → chain switch → destination-wallet → bridge. Cross-chain EVM↔Stellar trades now require (and surface) both wallets instead of throwing at runtime.SellAdd.tsxstub;TradeAdalready covers both directions.Test plan
pnpm --filter @proofbridge/backend-relayer test(unit + e2e)bash scripts/relayer-e2e/e2e.shagainst a local relayerpnpm --filter scripts/cross-chain-e2e start— EVM↔Stellar round tripTradeAdsurfaces the correct next step (connect pay / switch chain / connect destination / bridge).Summary by CodeRabbit
New Features
UI Updates
API Changes
dataand token responses includeassetIssuerandchainKind