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

request: Drop wrapper type PublicInput #311

Closed
aszepieniec opened this issue Jul 29, 2024 · 3 comments
Closed

request: Drop wrapper type PublicInput #311

aszepieniec opened this issue Jul 29, 2024 · 3 comments

Comments

@aszepieniec
Copy link
Collaborator

The wrapper type PublicInput is generating a lot of

  • lines of boilerplate code;
  • confusion whenever I try to create an object because I can never remember how;
  • type errors when functions do or do not expect an argument of this type and I made the wrong guess.

As far as I can tell, it is a simple wrapper around Vec<BFieldElement> and nothing else, and adds no functionality. It does arguably add a level of type safety against misuse -- for instance, redirecting a Proof to function that takes PublicInput generates an error. However, in my eyes the safety is not worth the hassle. I propose dropping the wrapper type altogether.

(And before you ask, yes the confusion and errors are painful because my IDE is not fast enough (probably due to all the macros) to make good automatic suggestions fast.)

@jan-ferdinand
Copy link
Member

There are good reasons why a dedicated type PublicInput is beneficial. A lesser one is maintainability: users of Triton VM should not have to care if the underlying implementation of PublicInput changes. A larger one is type safety: a compiler error is at least one order of magnitude better than a failing test, which itself is at least an order of magnitude better than a production failure. The test, you have to remember to write.

All that said, the points you raise should be addressed.

  • Boilerplate code and object creation: can you point me to a few places where PublicInput is constructed so I can try to see a pattern that might be better?
  • Type error due to confusion: this sounds like PublicInput is not used consistently in the places where it should be used. Do you know where it is used in spirit but not in type?

@aszepieniec
Copy link
Collaborator Author

aszepieniec commented Sep 9, 2024

Fails:

println!("standard input: {}", time_lock_witness.standard_input().iter().join(","));

also fails:

println!("standard input: {}", time_lock_witness.standard_input());

@aszepieniec
Copy link
Collaborator Author

5a52154 is the best of both worlds!

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