Simulate solana txs before send, fix peer calls#659
Conversation
- lock parameters to 3 items - introduce options object as 3rd parameter - add CallOptions interface - allow call and longCall methods to specify protocol in options object - refactor call sites: Sync.ts, and friends. - reroute peer.call to peer.httpCall for local node - simulate solana tx before broadcast
WalkthroughThis PR refactors network call infrastructure by unifying retry/timeout handling into an options-based API, enables TLSNotary and monitoring by default, improves logging consistency across consensus and network routines, enhances Solana transaction handling with simulation, and adds retry logic to peer bootstrap. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 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 unit tests (beta)
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. Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||||
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/features/multichain/routines/XMParser.ts (1)
67-91:⚠️ Potential issue | 🟠 MajorAlign execute() return type with new result shape.
You now store raw objects inresults[name], but the return type still promisesresult: string. This breaks the API contract and risks JSON serialization errors (e.g., BigInt). Update the type and log using the existingstringifyhelper (or revert to stringifying the stored result).✅ One possible fix (update type + safe logging)
- static async execute(fullscript: XMScript): Promise<{ - [operationId: string]: { - result: string - error?: string - } - }> { + static async execute(fullscript: XMScript): Promise<{ + [operationId: string]: { + result: unknown + error?: string + } + }> { const results = {} let name: string, operation: IOperation @@ - results[name] = result - log.debug("[RESULT]: " + results[name]) + results[name] = result + log.debug("[RESULT]: " + stringify(results[name]))src/features/multichain/routines/executors/pay.ts (1)
78-193:⚠️ Potential issue | 🟡 MinorNormalize Solana signed payload before deserialize.
The Solana handler uses
Object.values()directly, but other chains usevalidateIfUint8Arrayfor consistency. If the payload arrives as a hex string, already-typed array, or with out-of-order numeric keys,Object.values()can corrupt or misorder the bytes. Use the existingvalidateIfUint8Arrayhelper (imported at line 8) to handle all payload shapes robustly.Notably,
handleSolanaContractWriteincontract_write.tsalready delegates togenericJsonRpcPay, which usesvalidateIfUint8Arrayfor Solana—this handler should follow the same pattern.🛠️ Suggested change
- const payload = Uint8Array.from( - Object.values(operation.task.signedPayloads[0]), - ) + const payload = validateIfUint8Array( + operation.task.signedPayloads[0], + )src/libs/network/dtr/dtrmanager.ts (1)
329-343:⚠️ Potential issue | 🔴 CriticalBug: Wrong field name in
tryRelayTransactionparams - will not trigger handler.The handler in
manageNodeCall.ts(line 61) switches oncontent.message, buttryRelayTransactionusestype: "RELAY_TX"instead ofmessage: "RELAY_TX". This prevents the handler's RELAY_TX case from ever being triggered. Change line 334 fromtype: "RELAY_TX"tomessage: "RELAY_TX"to match the correct field name expected by the handler.
🤖 Fix all issues with AI agents
In `@src/libs/peer/Peer.ts`:
- Around line 279-442: In httpCall's catch generic branch (inside the async
method httpCall in Peer.ts) the 500 response returns the raw error object;
change this to return a serializable string by extracting a safe message (e.g.,
if axios.isAxiosError(error) use error.message or error.response?.data,
otherwise use String(error) or error.message) and put that string into the
response field (and optionally include a small serialized extra like { message:
..., code: error.code || null } ); update the return in the generic error branch
so response is always a string and extra contains only serializable primitives.
🧹 Nitpick comments (1)
src/libs/consensus/v2/routines/mergeMempools.ts (1)
6-31: Consider Promise.allSettled to avoid fail-fast merges.
If any peer call rejects, Promise.all aborts and skips partial successes. A settled aggregation keeps the merge resilient.♻️ Suggested change
- const responses = await Promise.all(promises) // ! Add error handling + const settled = await Promise.allSettled(promises) + const responses = settled + .filter( + (r): r is PromiseFulfilledResult<RPCResponse> => + r.status === "fulfilled", + ) + .map(r => r.value) + for (const r of settled) { + if (r.status === "rejected") { + log.error("[mergeMempools] Peer call rejected: " + r.reason) + } + }
| // Single HTTP call (no retry logic - use longCall for retries) | ||
| async httpCall( | ||
| request: RPCRequest, | ||
| isAuthenticated = true, | ||
| sleepTime = 1000, | ||
| retries = 3, | ||
| allowedErrors: number[] = [], | ||
| ): Promise<RPCResponse> { | ||
| let tries = 0 | ||
| let lastResponse: RPCResponse | null = null | ||
| log.info( | ||
| "[RPC Call] [" + | ||
| request.method + | ||
| "] [" + | ||
| new Date(Date.now()).toISOString() + | ||
| "] Making RPC call to: " + | ||
| this.connection.string, | ||
| ) | ||
|
|
||
| while (tries < retries) { | ||
| log.info( | ||
| "[RPC Call] [" + | ||
| request.method + | ||
| "] [" + | ||
| new Date(Date.now()).toISOString() + | ||
| "] Making RPC call to: " + | ||
| this.connection.string + | ||
| (tries > 0 ? ` (attempt ${tries + 1}/${retries})` : ""), | ||
| // Get some informations | ||
| const method = request.method | ||
| const currentTimestampReadable = new Date(Date.now()).toISOString() | ||
| // Prepare a request with our identity | ||
| let pubkey = "" | ||
| let signature = "" | ||
|
|
||
| if (isAuthenticated) { | ||
| const ourPublicKey = ( | ||
| await ucrypto.getIdentity(getSharedState.signingAlgorithm) | ||
| ).publicKey | ||
| const hexPublicKey = uint8ArrayToHex(ourPublicKey as Uint8Array) | ||
| const bufferSignature = await ucrypto.sign( | ||
| getSharedState.signingAlgorithm, | ||
| new TextEncoder().encode(hexPublicKey), | ||
| ) | ||
|
|
||
| // Get some informations | ||
| const method = request.method | ||
| const currentTimestampReadable = new Date(Date.now()).toISOString() | ||
| // Prepare a request with our identity | ||
| let pubkey = "" | ||
| let signature = "" | ||
|
|
||
| if (isAuthenticated) { | ||
| const ourPublicKey = ( | ||
| await ucrypto.getIdentity(getSharedState.signingAlgorithm) | ||
| ).publicKey | ||
| const hexPublicKey = uint8ArrayToHex(ourPublicKey as Uint8Array) | ||
| const bufferSignature = await ucrypto.sign( | ||
| getSharedState.signingAlgorithm, | ||
| new TextEncoder().encode(hexPublicKey), | ||
| ) | ||
|
|
||
| pubkey = getSharedState.signingAlgorithm + ":" + hexPublicKey | ||
| signature = uint8ArrayToHex(bufferSignature.signature) | ||
| } | ||
| pubkey = getSharedState.signingAlgorithm + ":" + hexPublicKey | ||
| signature = uint8ArrayToHex(bufferSignature.signature) | ||
| } | ||
|
|
||
| // REVIEW Using the connection string as the url with the new format | ||
| let connectionUrl = this.connection.string | ||
| // REVIEW Using the connection string as the url with the new format | ||
| let connectionUrl = this.connection.string | ||
|
|
||
| // INFO: If the peer is the local node, we use the internal connection string | ||
| if (this.isLocalNode) { | ||
| connectionUrl = getSharedState.connectionString | ||
| } | ||
| // INFO: If the peer is the local node, we use the internal connection string | ||
| if (this.isLocalNode) { | ||
| connectionUrl = getSharedState.connectionString | ||
| } | ||
|
|
||
| // Make the request | ||
| let timeoutId: NodeJS.Timeout | undefined | ||
| try { | ||
| // Create AbortController for connection timeout (covers TCP handshake + HTTP request) | ||
| const abortController = new AbortController() | ||
| timeoutId = setTimeout(() => { | ||
| abortController.abort() | ||
| }, 3000) | ||
|
|
||
| const response = await axios.post<RPCResponse>( | ||
| connectionUrl, | ||
| request, | ||
| { | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| identity: pubkey, | ||
| signature: signature, | ||
| }, | ||
| timeout: 3000, | ||
| signal: abortController.signal, | ||
| // Make the request | ||
| let timeoutId: NodeJS.Timeout | undefined | ||
| try { | ||
| // Create AbortController for connection timeout (covers TCP handshake + HTTP request) | ||
| const abortController = new AbortController() | ||
| timeoutId = setTimeout(() => { | ||
| abortController.abort() | ||
| }, 3000) | ||
|
|
||
| const response = await axios.post<RPCResponse>( | ||
| connectionUrl, | ||
| request, | ||
| { | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| identity: pubkey, | ||
| signature: signature, | ||
| }, | ||
| timeout: 3000, | ||
| signal: abortController.signal, | ||
| }, | ||
| ) | ||
|
|
||
| clearTimeout(timeoutId) | ||
| log.info( | ||
| "[RPC Call] [" + | ||
| method + | ||
| "] [" + | ||
| currentTimestampReadable + | ||
| "] Response received ", | ||
| ) | ||
| if (response.data.result == 200) { | ||
| log.info( | ||
| "[RPC Call] [" + | ||
| method + | ||
| "] [" + | ||
| currentTimestampReadable + | ||
| "] Response OK: " + | ||
| response.data.result, | ||
| ) | ||
| } | ||
|
|
||
| return response.data | ||
| } catch (error) { | ||
| // Clear timeout if request completed (error or success) | ||
| if (timeoutId) { | ||
| clearTimeout(timeoutId) | ||
| log.info( | ||
| } | ||
|
|
||
| // Handle abort/timeout errors | ||
| if ( | ||
| axios.isAxiosError(error) && | ||
| (error.code === "ECONNABORTED" || | ||
| error.message === "canceled" || | ||
| error.name === "AbortError" || | ||
| error.code === "ETIMEDOUT") | ||
| ) { | ||
| log.warn( | ||
| "[RPC Call] [" + | ||
| method + | ||
| "] [" + | ||
| currentTimestampReadable + | ||
| "] Response received ", | ||
| "] Request timeout/aborted to: " + | ||
| connectionUrl, | ||
| ) | ||
| // log.info(JSON.stringify(response.data, null, 2)) | ||
| if (response.data.result !== 200) { | ||
| log.warning( | ||
| "[RPC Call] [" + | ||
| method + | ||
| "] [" + | ||
| currentTimestampReadable + | ||
| "] Response not OK: " + | ||
| response.data.response + | ||
| " - " + | ||
| response.data.result, | ||
| ) | ||
| } else { | ||
| log.info( | ||
| "[RPC Call] [" + | ||
| method + | ||
| "] [" + | ||
| currentTimestampReadable + | ||
| "] Response OK: " + | ||
| response.data.result, | ||
| ) | ||
| } | ||
|
|
||
| lastResponse = response.data | ||
| this.markPeerOffline() | ||
|
|
||
| if ( | ||
| response.data.result === 200 || | ||
| allowedErrors.includes(response.data.result) | ||
| ) { | ||
| return response.data | ||
| } | ||
| } catch (error) { | ||
| // Clear timeout if request completed (error or success) | ||
| if (timeoutId) { | ||
| clearTimeout(timeoutId) | ||
| return { | ||
| result: 504, | ||
| response: "Request timeout", | ||
| require_reply: false, | ||
| extra: { | ||
| code: error.code || "ETIMEDOUT", | ||
| url: connectionUrl, | ||
| }, | ||
| } | ||
| } | ||
| // Handle ECONNREFUSED error | ||
| else if ( | ||
| axios.isAxiosError(error) && | ||
| error.code === "ECONNREFUSED" | ||
| ) { | ||
| log.warn( | ||
| "[RPC Call] [" + | ||
| method + | ||
| "] [" + | ||
| currentTimestampReadable + | ||
| "] Connection refused to: " + | ||
| connectionUrl, | ||
| ) | ||
|
|
||
| // Handle abort/timeout errors | ||
| if ( | ||
| axios.isAxiosError(error) && | ||
| (error.code === "ECONNABORTED" || | ||
| error.message === "canceled" || | ||
| error.name === "AbortError" || | ||
| error.code === "ETIMEDOUT") | ||
| ) { | ||
| log.warn( | ||
| "[RPC Call] [" + | ||
| method + | ||
| "] [" + | ||
| currentTimestampReadable + | ||
| "] Request timeout/aborted to: " + | ||
| connectionUrl, | ||
| ) | ||
|
|
||
| this.markPeerOffline() | ||
|
|
||
| lastResponse = { | ||
| result: 504, | ||
| response: "Request timeout", | ||
| require_reply: false, | ||
| extra: { | ||
| code: error.code || "ETIMEDOUT", | ||
| url: connectionUrl, | ||
| }, | ||
| } | ||
|
|
||
| if (allowedErrors.includes(504)) { | ||
| return lastResponse | ||
| } | ||
| this.markPeerOffline() | ||
|
|
||
| return { | ||
| result: 503, | ||
| response: "Connection refused", | ||
| require_reply: false, | ||
| extra: { | ||
| code: error.code, | ||
| url: connectionUrl, | ||
| }, | ||
| } | ||
| // Handle ECONNREFUSED error | ||
| else if ( | ||
| axios.isAxiosError(error) && | ||
| error.code === "ECONNREFUSED" | ||
| ) { | ||
| log.warn( | ||
| "[RPC Call] [" + | ||
| method + | ||
| "] [" + | ||
| currentTimestampReadable + | ||
| "] Connection refused to: " + | ||
| connectionUrl, | ||
| ) | ||
|
|
||
| this.markPeerOffline() | ||
|
|
||
| lastResponse = { | ||
| result: 503, | ||
| response: "Connection refused", | ||
| require_reply: false, | ||
| extra: { | ||
| code: error.code, | ||
| url: connectionUrl, | ||
| }, | ||
| } | ||
|
|
||
| if (allowedErrors.includes(503)) { | ||
| return lastResponse | ||
| } | ||
| } else { | ||
| log.error( | ||
| "[RPC Call] [" + | ||
| method + | ||
| "] [" + | ||
| currentTimestampReadable + | ||
| "] Error making RPC call:" + | ||
| error, | ||
| ) | ||
| log.error("CONNECTION URL: " + connectionUrl) | ||
| log.error("REQUEST PAYLOAD: " + JSON.stringify(request)) | ||
|
|
||
| lastResponse = { | ||
| result: 500, | ||
| response: error, | ||
| require_reply: false, | ||
| extra: null, | ||
| } | ||
|
|
||
| if (allowedErrors.includes(500)) { | ||
| return lastResponse | ||
| } | ||
| } else { | ||
| log.error( | ||
| "[RPC Call] [" + | ||
| method + | ||
| "] [" + | ||
| currentTimestampReadable + | ||
| "] Error making RPC call:" + | ||
| error, | ||
| ) | ||
| log.error("CONNECTION URL: " + connectionUrl) | ||
| log.error("REQUEST PAYLOAD: " + JSON.stringify(request)) | ||
|
|
||
| return { | ||
| result: 500, | ||
| response: error, | ||
| require_reply: false, | ||
| extra: null, | ||
| } |
There was a problem hiding this comment.
Return a serializable error string on generic failures.
The 500 branch returns a raw error object, which can be non-serializable and inconsistent with other responses. Prefer a string/message.
🛠️ Suggested change
- return {
- result: 500,
- response: error,
- require_reply: false,
- extra: null,
- }
+ return {
+ result: 500,
+ response:
+ error instanceof Error ? error.message : String(error),
+ require_reply: false,
+ extra: null,
+ }🤖 Prompt for AI Agents
In `@src/libs/peer/Peer.ts` around lines 279 - 442, In httpCall's catch generic
branch (inside the async method httpCall in Peer.ts) the 500 response returns
the raw error object; change this to return a serializable string by extracting
a safe message (e.g., if axios.isAxiosError(error) use error.message or
error.response?.data, otherwise use String(error) or error.message) and put that
string into the response field (and optionally include a small serialized extra
like { message: ..., code: error.code || null } ); update the return in the
generic error branch so response is always a string and extra contains only
serializable primitives.
1 participant
User description
PR Type
Bug fix, Enhancement
Description
Standardize peer call method signatures with CallOptions object
Simulate Solana transactions before broadcast to prevent failures
Fix XM operation result parsing by removing unnecessary stringify
Improve code formatting and logging consistency
Diagram Walkthrough
File Walkthrough
14 files
Improve XM execution result loggingAdd Solana transaction pre-flight simulationRefactor peer calls to use CallOptionsUpdate longCall invocations with options objectRemove unnecessary warning log messageRemove redundant warning logs and commentsRefactor longCall to use CallOptions objectUpdate longCall signatures and improve formattingRefactor DTR validator calls with CallOptionsChange warning log to debug levelUpdate adapter to use CallOptions interfaceStandardize call methods with CallOptions interfaceUpdate hello peer call with CallOptionsAdd retry logic and improve peer bootstrap1 files
Fix XM operation result parsing1 files
Re-enable TLS Notary and Monitoring featuresSummary by CodeRabbit
New Features
Bug Fixes & Improvements