-
-
Notifications
You must be signed in to change notification settings - Fork 68
fix: changes BTS transaction status on rejected in chat when node validation fails #857
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Deployed to https://msg-adamant-pr-857.surge.sh 🚀 |
1c54c5f
to
12a439b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add release notes please
return (failureCount: number): boolean => { | ||
return (failureCount: number, error: any): boolean => { | ||
// Don't retry BTC transactions on 404 (dust amount rejections) | ||
if (crypto === 'BTC' && error?.response?.status === 404) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
404 status is normal. Is it possible to check dust response?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And i think we need to do it in src/hooks/queries/transaction/useBtcTransactionQuery.ts
file
0d75e1b
to
273bd60
Compare
Co-authored-by: skranee <[email protected]>
retry: (failureCount: number): boolean => { | ||
// Don't retry dust amount BTC transactions | ||
const dustedIds = store.state.btc.dustedTransactionsIds || [] | ||
if (dustedIds.length > 0 && dustedIds.includes(unref(transactionId))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dustedIds.length > 0
is excessive
) | ||
context.commit('dustedTransactionsIds', signedTransaction.txid) | ||
} | ||
context.commit('transactions', [{ hash: signedTransaction.txid, status: 'REJECTED' }]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be it's better not to use dustedTransactionsIds
but something like
context.commit('transactions', [{ hash: signedTransaction.txid, status: 'REJECTED', isDust: true }])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was my first idea. But I paid attention that transactions from store show only on wallet page. On chat page list of transactions are fetching by API call. That's why I need to check each transaction from API response if it was rejected by dust amount.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, thanks.
@S-FrontendDev Please check if it's the optimal solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On chat page list of transactions are fetching by API call
Do we still store transaction info locally?
May be we can mark this tx as isDust
and amend this checking function accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamantmm
We discussed with @S-FrontendDev and decided to do this fix as part of task that already implements a proper transaction status synchronization system without workarounds - https://trello.com/c/S29m39O1/759-coin-tx-status-chore-fix-verify-update-coin-tx-sending-algorithm
I've created a new PR for this fix - #866
Co-authored-by: Izar Konti <[email protected]>
149baca
to
1efdacf
Compare
Successfully tore down https://msg-adamant-pr-857.surge.sh 🥲 |
Fixes illustration of status for BTC and BTC based coins transactions which are rejected due to dust amount on chat page