-
Notifications
You must be signed in to change notification settings - Fork 493
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
New opcode: base64decode #2961
Comments
Unnecessary. All our SDKs and APIs already do the work to have a binary-clean data path. |
It's needed because there are existing standards we want to support in
which a message contains b64 encoded fields, and then signed. The contract
needs to verify the message and then extract the data.
In particular, this is so we can handle messages for FIDO2 / WebAuthn,
described here:
https://medium.com/webauthnworks/verifying-fido2-responses-4691288c8770
…On Mon, Sep 27, 2021 at 12:09 PM Brian Olson ***@***.***> wrote:
Unnecessary. All our SDKs already do the work to have a binary-clean data
path.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2961 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADL7T34SDYTRTXJCYZHCXTUECJK7ANCNFSM5E25KOBA>
.
|
I do agree with Brian, however, that we should make it clear that this opcode has a very specific use case. DOn't go around b64 encoding your data that you send to a smart contract just because this exists. |
@jannotti @algoanne Should this PR aim to support all these with a user supplied flag? Thanks! |
Circling around my own question base64 has 2 alphabets in the RFC:
These are already present in go |
In the standard we are most interested in supporting, the alphabet used is called websafe-base64-encoded, which I imagine is that second one. I think it would be nice to support both though. You could look at the the verify_ecdsa to see how we normally deal with an opcodes that takes a distinguishing immediate argument to differentiate different versions. The the code those immediate are are often called "fields" (because the first and most common use is to fetch a band field is a transaction). |
There is some argument for not needing a field. Since those alphabets don't literally disagree on the meaning of any character, I suppose we could define our base64decode to work on either type, without the code needing to specify. (We would need to specify in order to support encode. But I don't think we should support encode) |
That makes sense. If we "autodetect" the encoding, we'd need to handle the case of ambiguity (basically when there is one of |
In order to avoid writing any decoding ourselves, and to make it easy to spec, if we go with the "TEAL decodes either alphabet" decision, I would recommend that the spec actually just says we try decoding with each alphabet in turn, until one passes or both fail. (And the implementation does likewise, which should give the behavior we'd want in your case of ambiguity - panic) |
In terms of implementation, I think your last approach is the cleanest. However, in terms of costs, we get a bit of extra non-determinism. I believe |
Create a new opcode,
base64decode
, that decodes base 64 inputs back into binary.note: how do we handle different base 64 alphabets? probably need to include a flag for which alphabet.
The text was updated successfully, but these errors were encountered: