feat: unify internal transfer transactions #1456
Conversation
euharrison
changed the title
| ); | ||
|
|
||
| // Currently we don't have the option of batching transfer transactions | ||
| if (txList.length === 0) { |
There was a problem hiding this comment.
Nitpick: let's prefer the use of the invariant? :)
| } catch (err) { | ||
| setGeneralErrorMessage(err + ""); | ||
| setCurrentStepIndex(0); | ||
| setTransaction(undefined); |
There was a problem hiding this comment.
After talking to @ChrisHoltDesign we won't have the timeline active on the "Signing" anymore, which simplifies A LOT of things. Maybe you can redirect directly to the transactions page after the storeTransaction just like I did on #1454 - we can do it in a later PR too
There was a problem hiding this comment.
Oh, this is a copy and paste from that you implemented 😁
I'll open a new PR focused on the Timeline 👍
| throw new Error("No gas config"); | ||
| } | ||
| invariant(sourceAddress, "Source address is not defined"); | ||
| invariant(chainId, "Chain ID is undefined"); |
There was a problem hiding this comment.
Maybe, just maybe, we could use a function to validate these variables here? We're repeating this validation multiple times
There was a problem hiding this comment.
Yeah, I agree, some variables on the begin of the component as well. I'll explore this more on the next PRs
|
I'll merge it and address the comments in the next PR :) |
Create the hook
useTransferwhich can be used to any Namada(internal) transfer transaction (not ibc, maybe yet?).Then, apply it to the
<NamadaTransfer>,<MaspShield>and<MaspUnshield>.Please note that this is the first step for #1297 as the notification implementation for
TransferandMasppage will be the same now, but they both still has some notification issues that should be handled in a different PR focused on the notification systems, not the sdk transaction