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

Add Provable to documentation #1972

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Conversation

45930
Copy link
Contributor

@45930 45930 commented Jan 17, 2025

Summary

Provable and Provable are both exported as Provable from the same file. There are good documentation comments on the constant variable Provable that we want to expose in the o1js-reference documentation, but it is tricky to disambiguate the two symbols.

Approach

This PR creates a dummy type called ProvableNamespace and exports it such that documentation about the type generates under type-aliases/ProvableNamespace. This isn't ideal, but it does quickly and without fuss get the documentation out there.

Alternatives considered

Modifying the export

- export { Provable };

+ export type Provable<t> = ...

+ export const Provabe = ...

We can abandon our style of declaring module exports at the top of the file, but the result is that many of the comments are missing in the ultimate documentation. For instance, this documentation is what we get for witness:

witness()

witness: <A, T>(type: A, compute: () => T) => InferProvable<ToProvable<A>>;

Type parameters

A extends ProvableType<any, any>

T extends any = From<ToProvable<A>>

Parameters

type: A

compute

Returns

InferProvable<ToProvable<A>>

And this is the documentation we get using the fake type workaround:

witness()

witness: <A, T>(type: A, compute: () => T) => InferProvable<ToProvable<A>>;

Create a new witness. A witness, or variable, is a value that is provided as input
by the prover. This provides a flexible way to introduce values from outside into the circuit.
However, note that nothing about how the value was created is part of the proof - Provable.witness
behaves exactly like user input. So, make sure that after receiving the witness you make any assertions
that you want to associate with it.

Example

Example for re-implementing Field.inv with the help of witness:

let invX = Provable.witness(Field, () => {
  // compute the inverse of `x` outside the circuit, however you like!
  return Field.inv(x);
}
// prove that `invX` is really the inverse of `x`:
invX.mul(x).assertEquals(1);

Type parameters

A extends ProvableType<any, any>

T extends From<ToProvable<A>> = From<ToProvable<A>>

Parameters

type: A

compute

Returns

InferProvable<ToProvable<A>>

The second version is what we want to show to developers.

Changing the name of one or the other Provable

- export { Provable };
+ export { ProvableT, ProvableV };

- type Provable<T> = ...
+ type ProvableT<T> = ...

- const Provable = ...
+ const ProvableV = ...

This approach seems like the best long-term solution, but involves fixing a lot of imports across the code-base and a potential for regression that my lightweight approach does not risk. I'd like to take this approach eventually, but first learn more about this code, why the names were chosen the way they were, and which footguns to look out for in extricating them.

@mitschabaude
Copy link
Collaborator

removing the docstrings from Provable seems to prioritize the online references over inline docstrings.

does this priority really reflect how developers work in practice? do they look up online docs more than just reading the docstrings from intellisense / by going to definition in the editor?
honest question, personally I almost never do the former and do the latter all the time when interacting with a library

@mitschabaude
Copy link
Collaborator

tricky to disambiguate the two symbols

Btw, TypeScript itself has no problems with that. docstrings in the editor are simply combined from both the type and the value definition.
Only typedoc seems to be confused by it 😕

@45930
Copy link
Contributor Author

45930 commented Jan 17, 2025

removing the docstrings from Provable seems to prioritize the online references over inline docstrings.

I don't intend the side effect of removing intellisense from Provable, that's a good catch. I can duplicate the comments so the show up in both places, and eventually I'd like to get rid of the dummy type and return to just having the site show the correct documentation.

I don't want to prioritize the online references over the inline docstrings, but I do want to prioritize the online references 😉

@45930
Copy link
Contributor Author

45930 commented Jan 17, 2025

Only typedoc seems to be confused by it 😕

This gave me the bright idea of just updating the typedoc version, and that seems promising. I will leave this PR in draft while I explore this option.

@45930 45930 mentioned this pull request Jan 17, 2025
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

Successfully merging this pull request may close these issues.

2 participants