Fix: warn if tx was already signed by connected wallet #2322
Conversation
Branch preview✅ Deploy successful! |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
| @@ -64,6 +65,8 @@ const SignForm = ({ | |||
|
|
|||
| return ( | |||
| <form onSubmit={handleSubmit}> | |||
| {hasSigned && <ErrorMessage level="warning">You have already signed this transaction.</ErrorMessage>} | |||
There was a problem hiding this comment.
Should we not move it into the CheckWallet component and add a new message there?
There was a problem hiding this comment.
CheckWallet doesn't know anything about transactions, it only checks the wallet itself.
We do, it's a destructive operation in the SDK. It mutates the signatures map. |
Oh nevermind my comment then. I'll approve :) |
| @@ -1,5 +1,5 @@ | |||
| .container { | |||
| margin-top: 42px; | |||
| margin-top: 10px; | |||
There was a problem hiding this comment.
The modal alignment no longer matches:
* Fix: mobile pairing check (#2327) * Fix: mobile pairing check * Fix tests * fix: align transaction links --------- Co-authored-by: katspaugh <[email protected]>
|
Not a big fan of allowing the user to sign again, but if is not harmful and we all agree is fine, then is ok for me. |
What it solves
Resolves #2321
Technically, nothing bad will happen if you sign a tx with the same owner twice. It will just overwrite the first signature.
But still good to warn about that.
However, I would keep the Sign button enabled because there can be a situation when a tx is signed locally, but sending it to the backend failed. In this case signing again is helpful.
In addition to that, some small stylistic adjustments in the tx flow.
How this PR fixes it
Shows a warning is the tx has already been signed by the connect wallet.
