feat: pass transaction id to status request#365
Conversation
| toChain: step.action.toChainId, | ||
| txHash, | ||
| ...(step.tool !== 'custom' && { bridge: step.tool }), | ||
| ...(step.transactionId && { transactionId: step.transactionId }), |
There was a problem hiding this comment.
You added transactionId to the getStatus call but the module-level cache TRANSACTION_HASH_OBSERVERS is still keyed only by txHash. This can cause cached responses to be reused across requests with different transactionId values. Consider including transactionId in the cache key or avoid module-level per-request caching.
Details
✨ AI Reasoning
A module-level cache (TRANSACTION_HASH_OBSERVERS) stores promises keyed by txHash. The PR adds transactionId into the request payload, so two requests with the same txHash but different transactionId will still reuse the same cached promise. This can leak or mix request-specific data between callers and cause incorrect status results. The problem was introduced/worsened by adding transactionId to the request without changing caching behavior.
🔧 How do I fix it?
Avoid storing request-specific data in module-level variables. Use request-scoped variables or explicitly mark shared caches as intentional.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
Which Linear task is linked to this PR?
Why was it implemented this way?
Explain the reasoning behind the implementation. Were there alternative approaches? Why was this solution chosen?
Visual showcase (Screenshots or Videos)
If applicable, attach screenshots, GIFs, or videos to showcase the functionality, UI changes, or bug fixes.
Checklist before requesting a review