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

Run-read-only native #273

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Run-read-only native #273

wants to merge 2 commits into from

Conversation

jmcardon
Copy link
Member

cc @CryptoPascal31

Implements KIP 0024.

The main difference is in the name. I felt that pure isn't really clear to users what it is, but run-read-only does. Moreover, pure has a different connotation in programming (a function that has no side effects), and also internally, the mode is called read-only in pact.

I'm still open to suggestions on the naming

PR checklist:

  • Test coverage for the proposed changes
  • PR description contains example output from repl interaction or a snippet from unit test output
  • (If Relevant) Documentation has been (manually) updated at https://docs.kadena.io/pact

Additionally, please justify why you should or should not do the following:

  • Benchmark regressions
  • Confirm replay/back compat (Ignore until core release)
  • (For Kadena engineers) Run integration-tests against a Chainweb built with this version of Pact (Ignore until core release)

@CryptoPascal31
Copy link

Thank you for implementing it... This will be a small but a significant step for improving the Pact security model.

The advantage of pure is that this is very concise: 4 letters. This is subjective, but IMHO it's important for "utility" natives.
And AFAIK in Pact, pure is synonym to read-only, as only DB writes can change the state of the Pact interpreter.

Maybe a tradeoff between concision and self-explanatory should be (read-only ) instead of (run-read-only )

@jmcardon
Copy link
Member Author

jmcardon commented Oct 31, 2024

The advantage of pure is that this is very concise: 4 letters. This is subjective, but IMHO it's important for "utility" natives.

This, I don't necessarily think is true. I do agree that this is definitely very subjective, and I don't necessarily think that the native needs to be short.

And AFAIK in Pact, pure is synonym to read-only, as only DB writes can change the state of the Pact interpreter.

This is true. It could be my personal bias here. pure to me means something different (no side effects, and given the compiler is written in haskell, i do have a bit of bias because pure is an entirely different function). I think I might be a bit nitpicky and pure is likely fine.

Maybe a tradeoff between concision and self-explanatory should be (read-only ) instead of (run-read-only )

I frankly prefer pure to read-only. run-read-only made sense to me because the expression e passed in to (run-read-only e) is evaluated in read-only mode, and thus run. I also envisioned as reading code that does this as:

(run-read-only 
  (do
    (thing)
    (thing2)
    (thing3))
 )

I think this reads pretty cool, it basically says "run this whole block of expressions in read only mode". It's almost self explanatory. I think the equivalent

(pure
 (do
   (thing)
   (thing2)
   (thing3))
)

Is likely fine as well (as much as it may confuse haskellers such as myself haha), it just didn't have the same ring. I am definitely amenable to renaming it and discussing it here or in the KIP, but preferably here.

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.

4 participants