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

verification function is not serializable #71

Closed
dckc opened this issue Jun 10, 2024 · 11 comments
Closed

verification function is not serializable #71

dckc opened this issue Jun 10, 2024 · 11 comments

Comments

@dckc
Copy link
Collaborator

dckc commented Jun 10, 2024

Passable does not include ordinary synchronous functions.

handleProofVerification comes from an E call; E calls can only return passables:

https://github.com/hindley-milner-systems/dapp-ertp-airdrop/blob/c8f8065744787bdd8490f9e57ae14f55ea373279/contract/src/airdrop/airdropKitCreator.js#L111C14-L114

No passable can be called synchronously like this:

handleProofVerification(proof, offerArgsInput.pubkey),

You could pass the source of the function as a string. But in office hours, you explained that the motivation for putting TreeRemotable in a separate vat was that a large amount of data was included in the definition of that function. We discussed some possible alternatives to that... but I suppose that should be a separate issue.

attn @tgrecojs (in case you're not automatically notified of every issue raised in this repo)

Copy link

linear bot commented Jun 10, 2024

@dckc
Copy link
Collaborator Author

dckc commented Jun 10, 2024

This issue will show up if you try to run this code on chain, such as in e2e tests normally used for #24

We also have a "bootstrap test" style, but we don't have much experience doing it outside the agoric-sdk repo.

cc @Jovonni

@tgrecojs
Copy link
Contributor

thanks for the continued attention here @dckc. it's sincerely appreciated 🙁

You could pass the source of the function as a string.

Can you elaborate on this approach?

But in office hours, you explained that the motivation for putting TreeRemotable in a separate vat was that a large amount of data was included in the definition of that function. We discussed some possible alternatives to that... but I suppose that should be a separate issue.

Indeed. I just created #72 to address the issue.

If you take a deeper look at the verification function, you'll find that the proof is also held within a remotable as well. Is this an issue? For context, the proof is an array, so I'm unable to pass it into the offer without serializing.

const proof = await E(offerArgsInput.proof).getProof();

@dckc
Copy link
Collaborator Author

dckc commented Jun 11, 2024

You could pass the source of the function as a string.

Can you elaborate on this approach?

It's not an option I would advocate. I suggest just importing the verification function as an ordinary module.

All I was pointing out is that if you have a function that you want to pass between vats, it's technically possible to take the source code of the function, pass it as a string (which, like all primitives, is passable) and then eval it on the other side. More likely Compartment.prototype.evaluate than eval. For example:

https://github.com/Agoric/agoric-sdk/blob/1dcae570fe7dc9d0ad3cda650b9209f392261d15/packages/vats/src/core/chain-behaviors.js#L83

@dckc
Copy link
Collaborator Author

dckc commented Jun 11, 2024

If you take a deeper look at the verification function, you'll find that the proof is also held within a remotable as well. Is this an issue?

Yes. Passing a remotable there would mean that getProof() is a call back to the party that submits the offer. But that call came from outside the VM, via a MessageSendAction. Slot references are resolved w.r.t. the board, so the proof remotable would have to be on the board. It's sort of theoretically possible, but horribly contorted (it would take another signed transaction, at a minimum).

The proof should be data. As you say...

For context, the proof is an array

so just putting that array right into the offerArgs seems most straightforward.

@tgrecojs
Copy link
Contributor

so the proof remotable would have to be on the board. It's sort of theoretically possible, but horribly contorted (it would take another signed transaction, at a minimum).

Just so we are clear, I didn't reach for a remotable right off the bat. And i'm of the same opinion wrt getProof being horribly contorted. Same goes for getVerificationFn.

Have you tried to pass an array into an offer as offerArgs @dckc ? Unless I'm experiencing unexpected behavior, it's prohibited. Doing so produces the following error message:

  Error {
    message: 'Cannot pass mutable typed arrays like {"data":[65,53,65,50,48,112,104,87,99,116,112,84,56,56,108,68,43,106,98,88,120,100,65,48,54,108,108,102,118,88,100,48,97,113,51,66,110,107,82,111,122,68,103,56,50],"type":"Buffer"}.',
  }

To be more specific, the shape of the proof array is below

[
        {
          data: Buffer @Uint8Array [
            41354132 30706857 63747054 38386c44 2b6a6258 78644130 366c6c66 76586430
            61713342 6e6b526f 7a446738 32
          ],
          position: 'right',
        },
        {
          data: Buffer @Uint8Array [
            b64eba9c 6b0c284b 9713129f eb4af01b a8faf514 5047fb5c 732823c8 3acb92f4
          ],
          position: 'right',
        },
        {
          data: Buffer @Uint8Array [
            43a06620 61d0c456 ed67b70d d702a3dd 03922852 3ea3ff51 e5124ea4 240deac7
          ],
          position: 'right',
        },
      ]

Even if it was an array of strings, then the contract would throw an error.

To recap, using Far alleviated my problems here as well as with the verification process, so I stuck with it and kept on my way. I promise you i'm not trying to make things more complicated than they already are. 🙃

@tgrecojs
Copy link
Contributor

tinker test with proof: [1] produces the following

ss_06112024_000374

@dckc
Copy link
Collaborator Author

dckc commented Jun 11, 2024

Have you tried to pass an array into an offer as offerArgs @dckc ? Unless I'm experiencing unexpected behavior, it's prohibited. Doing so produces the following error message:

  Error {
    message: 'Cannot pass mutable typed arrays like {"data":[65,53,65,50,48,112,104,87,99,116,112,84,56,56,108,68,43,106,98,88,120,100,65,48,54,108,108,102,118,88,100,48,97,113,51,66,110,107,82,111,122,68,103,56,50],"type":"Buffer"}.',
  }

That's a Buffer. Try a CopyArray; that is: a hardened Array.

@dckc
Copy link
Collaborator Author

dckc commented Jun 11, 2024

or a base64 encoded string

@endo/base64 is optimized on XS, IIRC.

@tgrecojs
Copy link
Contributor

That's a Buffer. Try a CopyArray; that is: a hardened Array.

Yes, my mistake. Did you see my comment regarding passing in a hardened array? It errors as well.

Sounds like I'll go with base64.

@dckc
Copy link
Collaborator Author

dckc commented Jun 11, 2024

That's a Buffer. Try a CopyArray; that is: a hardened Array.

Yes, my mistake. Did you see my comment regarding passing in a hardened array?

no; where?

It errors as well.

Got a full stacktrace?

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

No branches or pull requests

2 participants