fix(wallet): properly process ens transaction signals #16934
Conversation
dlipicar
requested review from
Cuteivist,
Khushboo-dev-cpp,
alaibe and
saledjenic
and removed request for
a team
Jenkins Builds
|
|
I tested it today and both issues are fixed |
| @@ -443,8 +443,8 @@ proc sendNotification[T](self: Module[T], status: string, sendDetails: SendDetai | |||
| addressFrom = sendDetails.fromAddress | |||
| addressTo = sendDetails.toAddress | |||
| txTo = "" # txFrom is always the same as addressFrom, but txTo is different from addressTo when the app performs any sending flow via certain contract | |||
| fromChain = sendDetails.fromChain | |||
| toChain = sendDetails.toChain | |||
| fromChain = sentTransaction.fromChain | |||
There was a problem hiding this comment.
I could swear that I've added those two params to SendDetails on the statusgo side, not sure when/how they disappeared (could be while rebasing/preparing the final PR for review). Anyhow, accessing sentTransaction here is a dangerous cause that can be nil in some cases and that will lead to a crash.
Why don't we just update statusgo and add those 2 fields to SendDetails cause that was my intention when was working on that.
The point is to initially set from and to chains based on SendDetails and later in this function if sentTransaction is provided to update those details and emit signal for notification.
Let's please update statusgo with that and revert all changes except one in src/app/modules/main/profile_section/ens_usernames/module.nim.
There was a problem hiding this comment.
ugh there's already so much duplicated data between the two structs. What's the reason for having each field twice?
I see this and I think "which of the two fields should I trust?"
There was a problem hiding this comment.
SendDetails are always sent from statusgo, while tx is being sent only if the tx succeeds/exists and that's why I added (but somehow lost later) from/to chains to SendDetails.
SendDetails expresses the intention of sending, while RouteSentTransaction carries data about each individual tx.
For example when swap/bridge send details will have chains X and Y as "from" and "to" chains, while related approval tx will have X for both "from" and "to" chains and swap/bridge tx will have X for "from" and Y for "to" chain.
What does the PR do
Fixes #16921 and #16922
SendDetailsdoesn't have chain information, that comes with eachRouterSentTransactionThis PR takes the chainID info from the proper field, indirectly fixing ENS name events and potentially other bugs.
This fix uncovered a new bug that re-added an ENS name to the list after a removal due to triggering a release. This is fixed in the second commit.