Clarity should permit local binding of global symbols

[njordhov]

Clarity should permit local binding of symbols having global definitions, making Clarity contracts more future-proof and avoiding the sacrifices and complications otherwise incurred.

Currently, contracts with local bindings of globally defined symbols can be deployed but will trigger a NameAlreadyUsed error during evaluation.

Case in point, say we deploy a function part with a local binding of the symbol slice:

(define-read-only (part)
  (let ((slice "piece of cake"))
    slice))

This contract runs fine as is, but fails after introducing a function named slice. As is, every new function added to the language is a breaking change, requiring special handling of existing contracts. Stacks 2.1 will introduce a native slice function, and issue #3131 proposes a complicated workaround to avoid breaking existing contracts.

But even having a breaking version of Clarity for each new native function won't alleviate all issues. For example, the contract above would fail deployment in Stacks 2.1 due to its native slice function. Moreover, developers (and software) cannot confidently reuse a function like part from an existing contract. Clarity code examples will have to be vetted for each new Clarity version to ensure there are no conflicts with newly introduced global symbols. And so on.

Not allowing local binding of symbols with global definitions also complicates generating readable Clarity code. To avoid potential conflicts with global definitions, code generators may avoid using meaningful names for local bindings, instead opting for cryptic symbol names as in this obfuscated contract.

The fix is to remove the evaluation restriction on local bindings of globally defined symbols.

Related:
Issue #2696
Issue #3176
Issue #3131

[jcnelson]

The consensus from #3176 is that all Clarity keywords should be reserved, and unbindable (including ones that are not reserved today). The rationale is to ensure that code remains readable. Developer cleverness and flexibility can, in general, be sacrificed to ensure that the code is easier for other people to read (since the code can manage digital assets that have non-trivial real-world value).

[njordhov]

@jcnelson There is no such consensus in #3176. The readability benefits of reserving keywords are minor and dwarfed by the negative consequences, including that adding a new native Clarity function is a breaking change requiring complex workarounds like #3131 for the VM to handle concurrent versions of Clarity. Reserving keywords complicates code maintenance and sacrifices the security benefits of a simpler VM implementation for dubious gains in readability.

[jcnelson]

@njordhov Respectfully, you seem to be the only one who wants this feature. None of the other folks who regularly contribute to this repo want this change, for the reasons outlined above.

Even if making it possible for local binding of global symbols was a universally desired feature, doing so would not obviate #3131. This is because some of the new keywords introduced in Clarity2 either (1) access state that the VM currently cannot access (e.g. get-burn-block-info? does this), or (2) perform type analysis that the Clarity type system does not support (e.g. from-consensus-buff does this). So either way, there will need to be at least two versions of Clarity.

[njordhov]

@jcnelson This is not a request for a "feature" but about correcting a severe flaw in the technical architecture.

Adding new keywords to Clarity shouldn't cause a breaking version of the language. If we don't reserve keywords but allow rebinding, the names of new native functions won't conflict with symbols used in existing contracts, enabling legacy contracts to be evaluated the same as contracts calling the new functions.

Do you have a concrete example of Clarity1 contract code that would no longer evaluate after introducing get-burn-block-info? or from-consensus-buff due to the reasons you proclaim?

[jcnelson]

Adding new keywords to Clarity shouldn't cause a breaking version of the language

There is no breaking change to the language. All Clarity1 code will continue to be supported indefinitely, since all future versions of the blockchain must be able to process the existing chainstate the exact same way as it currently does. Moreover, with #3131, developers can choose which version of the VM will be used to evaluate their code (regardless of the versions of the other contracts that call into it).

However, the blockchain will need to perform a coordinated breaking change, because the VM will suddenly support new keywords that would be valid in Clarity2, but are invalid today.

Do you have a concrete example of Clarity1 contract code that would no longer evaluate after introducing get-burn-block-info? or from-consensus-buff due to the reasons you proclaim?

#3131 makes it so that even if a Clarity1 contract defined a function called get-burn-block-info?, it would still run as the developer intended. Also, a Clarity2 contract would be able to call it, and the code body would be evaluated according to the Clarity1 rules. This is extensively tested in #3131.

EDIT: What happens now is that the VM tracks which version of Clarity to use when evaluating a particular function code body. All contracts today are Clarity1 contracts, and follow Clarity1 evaluation rules. This means that a Clarity1 contract that binds a reserved keyword in Clarity2 will continue to work, because in Clarity1 rules, the Clarity2 keywords are not recognized as keywords. Clarity2 contracts can call into Clarity1 contracts, and vice versa, but the function code body will be evaluated according to the function's contract's Clarity version. So, a Clarity2 contract can call a Clarity1 contract function called get-burn-block-info?. This is allowed:

;; Clarity1 contract named `.test`
;; You can deploy this contract even in Stacks 2.1, as long as the transaction payload indicates that this
;; meant to be instantiated as a Clarity1 contract.
(define-public (get-burn-block-info?) (ok true))
(define-public (call-get-burn-block-info?)
   (get-burn-block-info?))

;; Clarity2 contract
(begin
   (contract-call? .test get-burn-block-info?)
   (contract-call? .test call-get-burn-block-info?))

[njordhov]

the VM will suddenly support new keywords that would be valid in Clarity2, but are invalid today.

It's actually the other way around: The Clarity VM will suddenly reserve keywords that are valid today.

with #3131, developers can choose which version of the VM will be used to evaluate their code

With all due respect, there has been no consensus on this among Clarity contract developers, most of whom are unlikely to have any idea yet they will have to juggle multiple versions of the language.

This is extensively tested in #3131.

The complexity added by #3131 would make the implementation harder to maintain and increase the chance of bugs. Complex code such as in PR #3192 are likely to have bugs, possibly severe ones.

By avoiding or reducing unnecessary code complexity, we can improve maintainability and decrease the chance of bugs.

There is no breaking change to the language.

Well, say a currently deployed Clarity contract defines a trait with a function get-burn-block-info?:

(define-trait burn-block-info
  ((get-burn-block-info? () principal)))

Now, as Clarity2 introduces a built-in get-burn-block-info? function, as long as it is a reserved keyword, the burn-block-info trait cannot be implemented in a Clarity2 contract.

However, my point was that with reserved keywords, adding a new native Clarity function is a breaking change requiring complex workarounds like #3131 for the VM to handle concurrent versions of Clarity. Fortunately, the S-expression syntax of Clarity makes it unnecessary to reserve keywords, which opens for simplifying the VM instead of making the implementation more complex and Clarity development more complicated.

[jcnelson]

With all due respect, there has been no consensus on this among Clarity contract developers, most of whom are unlikely to have any idea yet they will have to juggle multiple versions of the language.

The technical CAB voted overwhelmingly to implement the following in Stacks 2.1:

• There will be two supported versions of the VM: Clarity 1 and Clarity 2. Developers can choose which VM (and which language rules) will process their contracts.

• The fact that Clarity 2 reserves new keywords is not a concern in practice, because Clarity 2 can call into Clarity 1 contracts and vice versa. Also, to date, there are no trait definitions which use Clarity 2 keywords. So the impact on developers is de-facto nil.

Furthermore, the ongoing SIP-015 vote is overwhelmingly "yes".

The complexity added by #3131 would make the implementation harder to maintain and increase the chance of bugs. Complex code such as in PR #3192 are likely to have bugs, possibly severe ones.

Making a variant of the Clarity VM which permits local binding of global symbols would also have this effect, so it's not clear that there's any gain here. In general, changing the semantics of the VM requires a breaking change and a new variant of the VM to be implemented, because the VM's evaluation of the transaction code is necessarily on the critical path for block validation. The unavoidable consequence of requiring that the developer's hand-written code be the code that gets interpreted upon transaction execution is that a change to the VM's behavior necessarily changes the calculation of the block's state root hash. Meaning, there exists a block such that if two different VM variants evaluate this block, they will calculate two different state root hashes (of which at most one can be valid). In addition, permitting contract code to change the meaning of language built-ins would create a means for nefarious actors to obfuscate their code, which we don't want.

Anyway, I consider this issue to be closed, given that the CABs and STX voters have already affirmed the current system design.

[blockstack-devops]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.