Skip to content
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

Shielding does not automatically reveal a pk #3552

Closed
sug0 opened this issue Jul 25, 2024 · 4 comments · Fixed by #3594
Closed

Shielding does not automatically reveal a pk #3552

sug0 opened this issue Jul 25, 2024 · 4 comments · Fixed by #3594

Comments

@sug0
Copy link
Collaborator

sug0 commented Jul 25, 2024

When you attempt to shield with an account whose pk hasn't been revealed yet, the multitoken VP and masp VP both fail with a "signature threshold not met" error.

The returned client message isn't too helpful in figuring out the cause of this error. We should either:

  1. Automatically send a reveal pk transaction along with the shielding transaction, or...
  2. State the reason of the failure (in this case, the public key hasn't been revealed yet, thus it's missing in storage).

Example flow

https://pastebin.com/GSRdwyTm

@sug0 sug0 added the UX label Jul 25, 2024
@brentstone brentstone added this to the To evaluate milestone Aug 6, 2024
@brentstone brentstone mentioned this issue Aug 8, 2024
2 tasks
@brentstone
Copy link
Collaborator

brentstone commented Aug 8, 2024

Included the auto reveal-pk for shield in #3594 at bbd5a6d.

But another question arises: should we be revealing the pk for all source addresses that do not have their pk revealed yet, since transfers now support multiple sources? This would apply for shield and transparent-transfer as well. Right now we only consider the first source I believe.

@grarco @sug0

@sug0
Copy link
Collaborator Author

sug0 commented Aug 8, 2024

Since we went with option 1), I guess that should be the default behavior for all source addrs.

@mergify mergify bot closed this as completed in #3594 Aug 9, 2024
@grarco
Copy link
Collaborator

grarco commented Aug 10, 2024

An additional note, I believe since we have tx batches now we could pack all the reveal-pk transactions and the desired tx together and submit a single batch, I've mentioned this thing here #1879 (comment)

@Fraccaman
Copy link
Member

yeah agree, this is what the interface team is doing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants