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

SIP - Contract Documentation Standard #32

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lgalabru
Copy link
Contributor

@lgalabru lgalabru commented Aug 5, 2021

Creating this SIP as a draft to collect ideas and feedbacks.
What's our take on standardizing contract documentation?
If we were sharing a standard (a la doxygen or something), like:

;; @contract Counter contract 
;; @version 1

(define-fungible-token tok ...)

;; @desc increment the internal counter
;; @param a; value of the increment 
;; @post stx; will be transferred to contract
;; @post tok; will be minted for new owner
(define-public (increment (a uint))
    ...)

Then I think that the overall experience for users interacting with contracts could be enhanced, by explaining what will be done with args and post conditions.
Ideally, if we can converge on a convention, then the stacks-api could handle the mapping, and expose a documented ABI, that wallet can consume.

Thoughts?

@philiphacks
Copy link

Agree with this. Arkadiko is following this convention on all their public functions.

@friedger
Copy link
Contributor

friedger commented Oct 5, 2021

The sip should contain a specification about documenting a minified contract, in particular about adding a reference to a fully documented version of the contract. That would imply a standardization of minification. Is that out of scope?

@obycode
Copy link
Contributor

obycode commented Nov 7, 2021

I agree this would be very valuable. I'd love to see something included that would allow wallets and explorers to show details about errors as well. For example, something like:

;; @err Balance too low
(define-constant balance-too-low-err (err u67))

This could then be used to show "Error: Balance too low" instead of just showing "(err u67)".

@friedger
Copy link
Contributor

friedger commented Nov 8, 2021

Instead of storing this data on-chain, a

@error-uri "https://..."

would be more efficient, and would better support localization.

@obycode
Copy link
Contributor

obycode commented Nov 8, 2021

I do like the idea of having the contract point off-chain for detailed documentation, but what would be the argument for having some structured on-chain documentation and some off-chain? Is that because we could consider the description, parameter explanation, and post conditions vital to the security, but the error message as just a convenience?

@criadoperez
Copy link

If the documentation is stored to a link off-chain, the documentation can change independently of the contract.
This has positive and negative aspects. On one hand, it gives you more flexibility but on the other it also allows a malicious user to alter the documentation with misguided information without touching the contract.
A way to solve this can be to store in chain a hash of the documentation, to guarantee that the documentation is valid for that contract.
Not sure if you need to go down that road. Just thinking out-loud here about possible implications...

@jcnelson
Copy link
Contributor

If the documentation is stored to a link off-chain, the documentation can change independently of the contract.

The hash(es) of the documentation could be stored on-chain if this is a concern.

@obycode
Copy link
Contributor

obycode commented Nov 16, 2021

We should consider this topic together with the suggestion in stacks-network/stacks-core#2926 which discusses storing a minified version of the contract. If we decide to do that, then it would reduce the cost of including significant amounts of comments directly in the contract.

@markmhendrickson
Copy link

We'd be very interested in leveraging the data that conforms to such a standard in the Hiro Wallet during transaction signing to better present and explain the contract being signed. Currently, the contract name and arguments alone can be quite cryptic for non-technical users.

@jcnelson jcnelson changed the title SIP 014 - Contract Documentation Standard SIP - Contract Documentation Standard May 16, 2022
@jcnelson
Copy link
Contributor

We should consider this topic together with the suggestion in stacks-network/stacks-core#2926 which discusses storing a minified version of the contract.

To be clear, stacks-network/stacks-core#2926 refers to compression, not minification. I want to emphasize that the compression algorithm will not attempt to parse the contract code body.

@hugocaillard
Copy link

I suggest that we use a specific notation for doc comments (such as ;;; instead of ;;)
It would allow some doc specific features such as scaffolding of docs section / auto-completion and would make syntax highlighting easier.

Just like JSDoc uses /** */ instead of /* */

@friedger
Copy link
Contributor

friedger commented Oct 3, 2023

We had a discussion about ;; and ;;; where the latter is used for docs that can be removed before deployment for cheaper costs.

@hugocaillard
Copy link

hugocaillard commented Oct 3, 2023

I think I could find some use case where it would be useful to have it on mainnet. But yes, if deployment costs are an important consideration, we could have an option to remove docs comment on deploy

@friedger
Copy link
Contributor

friedger commented Oct 3, 2023

I am all for a reasonable documentation on-chain.

How to represent

  • on chain docs, including docs for doc-gen
  • off-chain docs, including docs not for doc-gen

@hugocaillard
Copy link

@friedger I see, I didn't consider "off-chain docs, including docs not for doc-gen"
Not sure if it's in the scope of this of this SIP but definitely something to take into consideration.

Anyway, I'm in favor of using a special comment notation for on chain docs, including docs for doc-gen

@friedger
Copy link
Contributor

friedger commented Oct 3, 2023

Ludo suggested keywords starting with @ for docs-gen.

Combined with ;;; we could cover all cases.

@hugocaillard
Copy link

yeah that could work but I'm not sure it would be perfect. I think it would be easier to have the discussion on a PR comments or on Discord rather than in the PR main thread

@obycode
Copy link
Contributor

obycode commented Jul 9, 2024

Noting some discussion items here from the Clarity WG call:

  1. We should revitalize this SIP and get it formalized
  2. With the coming Clarity Wasm, only the built wasm module needs to be loaded on function calls, not the contract source, so the cost of having extra comments should only affect deployment, not every call to the contract.

wjorgensen added a commit to s-alad/Raized that referenced this pull request Aug 31, 2024
…e more readable, Changed documentation to fit this SIP stacksgov/sips#32
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.

8 participants