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

Utility package to specify general function conditions #301

Open
lumtis opened this issue Jul 23, 2022 · 3 comments
Open

Utility package to specify general function conditions #301

lumtis opened this issue Jul 23, 2022 · 3 comments

Comments

@lumtis
Copy link
Contributor

lumtis commented Jul 23, 2022

I'm opening this issue assuming this is not a planned feature. Maybe you already discussed privately about this.
 

Problem

 
With smart contracts, functions may have general conditions:

  • Read only, should not be called in a tx
  • Tx that expect gnot to be sent (payable)
  • Tx that doesn't expect gnot to be sent
  • Should not be called by other package, only by sender (external)
     
    Such conditions allow to prevent sending tx for action that doesn't perform any state change, prevent sending funds to a package where those would be locked forever, etc... and therefore having a conventional solution to represent these conditions may improve developer and user experience. A convention can simplify building clients with better UX (preventing calling render through a tx for example)
     
    Solidity includes function types in its own syntax as keywords to allow to set such conditions: payable, view, external, etc...
     
    AFAIK the philosophy is to keep Gno syntax as simple and as close to Go as possible and therefore such keywords can't be incorporated into the interpreter.
     

Proposed solution

 
I may see the method std.AssertOriginCall() that already exists and is apparent to an "external" condition.
 
Therefore, a solution could be to extend the assert available methods from the standard package. Or defining a new package for this purpose. For example, a standard package named "rules"
 

import (
  "rules"
)
 
func Foo() {
  rules.Payable() // panic if no send tokens
  rules.ReadOnly() / panic if called in a tx
 
  //...
}

 
But a procedural style to define these rules might be less adapted than a declarative style. Declarative style is more visual for listing a set of conditions and makes building a code parser easier, for a client for example.
 
So an even better solution IMO can be to define a method taking generic rules. For example, an Apply method
 

// check a condition
type Rule func()
 
// check all listed condition
func Apply(rules ...Rule) {
  for r range rules {
    r()
  }
}
 
var Payable Rule  = func() = { /*check send tokens not empty*/ }
var NonPayable Rule  = func() = { /*check send tokens empty*/ }

 
Conditions can be defined as following
 

import (
  "rules"
)
 
func Foo() {
  rules.Apply(
    rules.Payable,
    rules.NonPayable,
    rules.ReadOnly,
    rules.External,
    rules.PayableWith(["1000ugnot"]),
    rules.AdminOnly(["gxxx"]),
    //...
  )
 
  //...
}

 
This example simplifies implementing a generic client that parse the AST of the uploaded smart contract and shows in the UI the methods differently, prevent calling a ReadOnly method in a tx for example.

@moul
Copy link
Member

moul commented Aug 2, 2022

This is an excellent idea!

We should start writing helpers in gno.land/p/something (not rules, can be too many different things) and consider making a top-level package only after encountering limitations with this method.


BTW, I prefer the procedural over declarative.

My 2cts:

  • declarative makes more ambiguous stack traces.
  • for listing a set of conditions and makes building a code parser easier -> I disagree. It's straightforward to parse in both cases.
  • less indirections.

If you're concerned about readability, we can use idiomatic go like this:

func Foo() {
    {
        rules.AssertPayable()
        rules.AssertReadOnly()
        rules.AssertPayableWith("...")
        // ...
    }

    // ...
}

But I think keeping them at the top of functions with an empty line just after will be more than enough.


The good news is that we can manage to support both at the same time, but I'll prefer the procedural method for the official packages' implementation.

@lumtis
Copy link
Contributor Author

lumtis commented Aug 14, 2022

Sounds good to have a package in p for this. I was wondering if it would possible to implement ReadOnly through this method since it would be something quite useful.
For procedural vs declarative, I guess it's personal preference, using brackets is actually already visual.

@moul
Copy link
Member

moul commented Oct 18, 2022

I think a more go-ish approach would be to use comments to give more context to the clients, generate documentation, and so on.

It's the current way Go extends a source code for things that are for tools and humans.

Having a library for rules still makes sense for real assertions, i.e., I would not prevent people from making TXs for read-only stuff, a TX is more decentralized and uses the consensus + it will be IBC friendly.
So, for the example of read-only, I would add comments to suggest people call it using RPC instead of TX but not prevent it.

Also, mixing actual code and magic is more obscure than having explicit code for code and explicit comments for everything else non-code.


I suggest that anyone starts a rules package that will make sense. And consider parsing comments instead of code for everything related to annotations/suggestions.

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

No branches or pull requests

3 participants