-
Notifications
You must be signed in to change notification settings - Fork 967
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
Avoid redundant wrapper signatures #3896
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3896 +/- ##
=======================================
Coverage 73.24% 73.25%
=======================================
Files 341 341
Lines 105240 105328 +88
=======================================
+ Hits 77085 77156 +71
- Misses 28155 28172 +17 ☔ View full report in Codecov by Sentry. |
b0df0e7
to
5c08510
Compare
@tzemanovic seems like we have a failure in the speculos test, could be because of the extra function called now for the signature but I'm not sure |
Yeah, it is that, but it's kinda odd - the first tx with bond has to be signed twice, but all the other txs (redeleg, unbonding and withdrawing) in that test have not changed. Edit: the first tx is signed twice because of batched reveal tx, right? In the Ledger wallet it looks like it's just signing the same bond twice, maybe we can print something the client to make this clear (we have a similar UX issue in #3797). |
@tzemanovic I'm closing this as in hindsight I believe it's better to pursue a cleaner solution such as #3883 (even if SDK breaking) rather than this non-breaking workaround |
Describe your changes
Temporary, non-breaking workaround for #3883.
Adds an SDK function to remove the duplicated wrapper signatures that might come from
build_batch
. Updatesbatch_opt_reveal_pk_and_submit
to call such function.Checklist before merging
breaking::
labelsnamada-docs
reponamada-indexer
ornamada-masp-indexer
, a corresponding PR is opened in that repo