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: Optimistic Dispatch and Execution Plan Commitment #176

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

Conversation

lgalabru
Copy link
Contributor

@lgalabru lgalabru commented Mar 19, 2024

This SIP reevaluates the existing approach and proposes a new design that gives developers more freedom in assembling their execution plan (a set of dynamic contract IDs to invoke), while also providing users with a method to verify, after execution, that the executed plan matches with what they committed to.

Consequently, if the execution plan changes between the time a user signs and broadcasts their transaction and the time the transaction is executed, the said transaction would be rolled back, similar to a rollback triggered by a post-condition failure.

The concept of Optimistic Dynamic Dispatch is defined as follows: by default, the smart contract is trusted to correctly route the contract calls. Additionally, a mechanism is provided for users to have the transaction rolled back if the execution ends up running contracts that were not initially specified.

@hstove
Copy link
Contributor

hstove commented Mar 22, 2024

Hey Ludo, thanks for putting time into this!

I understand the motivation, but I don't think the current form of this SIP does a good job of demonstrating what the concrete changes would be.

For example, with this change, is the idea that you could invoke a contract call based on dynamic state? Something like this:

(define-data-var fn-name (string-ascii 100) "some-fn")
(define-data-var contract principal .contract-name)

(define-public (dynamic-dispatch (var1 uint))
  (contract-call? (var-get contract) (var-get fn-name) var1)
)

Additionally, I think dynamic dispatch is mainly useful with contract factories. For example, imagine an AMM. Given two tokens, you could deterministically construct the expected contract of the pool, but with Clarity someone still has to deploy that. And without factories, you don't have any way to ensure that the dynamically constructed contract has the exact code you expect. Without Clarity having contract factories, I find this proposal only partially achieves the utility of dynamic dispatch.

Regarding that last point, if there was a Clarity function that returns the sha256 of a given contract's code, you could achieve my example with trust about the contract's code, even without factories.

Here's an example contract that would utilize this hypothetical contract-hash function, along with dynamic dispatch, to have an AMM with pools that anyone can deploy:

;; mapping of token pools
(define-map pool-contracts { a: principal, b: principal } principal)

(define-constant POOL_CODE_HASH 0xdeadbeef)

;; anyone can add a new pool contract. the pool's code must exactly hash
;; to POOL_CODE_HASH
(define-public (add-pool (token-a principal) (token-b principal) (pool principal))
  (begin
    (asserts! (is-eq (contract-hash pool) POOL_CODE_HASH) (err "invalid pool contract"))
    (ok (map-set pool-contracts { a: token-a, b: token-b } pool))
  )
)

(define-public (swap (token-a principal) (token-b principal) (amount uint))
  (let
    (
      (pool (unwrap! (map-get? pool-contracts { a: token-a, b: token-b }) "pool not found"))
    )
    (contract-call? pool swap token-a token-b amount)
  )
)

@lgalabru
Copy link
Contributor Author

@hstove thanks for the feedbacks, happy to re-work this SIP or to co-write it with you if you're feeling inspired :).

(define-data-var fn-name (string-ascii 100) "some-fn")
(define-data-var contract principal .contract-name)

(define-public (dynamic-dispatch (var1 uint))
  (contract-call? (var-get contract) (var-get fn-name) var1)
)

Bingo, this is exactly the kind of patterns this SIP would unblock, while maintaining Clarity decidability.

Contract factory would be great I agree. I remember having a technical discussion about this feature in the early days of Stacks 2.0. I'm pretty sure we were able to reach positive consensus on their utility and feasibility, however I think it'd need to be a dedicated SIP - it's a different beast.

@AcrossfireX
Copy link

@lgalabru - would you want to discuss this on one of our upcoming SIP editor spaces to get this idea in front of more people? We typically host these at 3PM ET every other Thursday on X. Let me know if you would like to explain this idea to the community or get support for helping your idea along.

@jcnelson
Copy link
Contributor

jcnelson commented Mar 27, 2024

Bingo, this is exactly the kind of patterns this SIP would unblock, while maintaining Clarity decidability.

Decidibility would only be maintained if the execution path is always committed to in advance. If this was not the case -- if there was no commitment to the execution path -- then Clarity would be rendered undecidable in this proposal. This is because the value of the contract address loaded from the data space could be the result of a computation running through an arbitrary, unbounded sequence of Clarity contract-calls. While the Clarity computation performed by individual contract-call or contract-deploy transaction is decidable, an arbitrary unbounded sequence of such transactions would enable someone to implement unbounded loops and unbounded recursion -- each transaction could be a single loop-pass or function-evaluation, but the state preserved at the end of such a transaction would be the continuation state of the next transaction. So, you wouldn't know what contract you're jumping to until you actually tried it. Even then, this proposal doesn't require any typing information about the principal, so you wouldn't even know whether or not you're even calling a contract, let alone an existing method with a type-compatible signature.

If decidability is to be maintained, the commitment to the execution path is required. However, this is the problem that traits solve today -- by requiring the contract-call to pass in a trait concretization as an argument, the caller will have committed to a specific execution path embodied by the trait implementation. Moreover, unlike this proposal, the entire call path would be statically analyzed before execution begins, thereby ensuring that all contracts invoked during the transaction's execution (1) exist, and (2) have the expected type signatures.

In @hstove's example, the swap function would require a trait implementation for pool, which the caller would have to provide. In order to implement a whitelist of pools, you could use contract-of to determine if the trait implementation is legitimate. For example:

The trait (deployed as .foo):

(define-trait foo
    (
        (lolwut () (response bool uint))
    )
)

The implementation (deployed as .foo-impl):

(impl-trait 'SP1V4XDF7WYFQ829GWZ0D3BJ41E2R16X9JRK945AR.foo.foo)

(define-public (lolwut)
    (ok true))

The whitelist check:

(use-trait foo 'SP1V4XDF7WYFQ829GWZ0D3BJ41E2R16X9JRK945AR.foo.foo)

(define-public (whitelisted? (foo-impl <foo>))
   (if (not (is-eq (contract-of foo-impl) 'SP1V4XDF7WYFQ829GWZ0D3BJ41E2R16X9JRK945AR.foo-impl))
       (err "wrong trait impl")
       (ok true)))

With principal-destruct?, you could go even further and verify that the trait implementation was published by an authorized developer, so you'd only need to check the principal.

In @hstove's example, the add-pool function would be used to populate a set of whitelisted set of pool publishers, and they could publish contracts that conform to a pool trait. That pool trait itself could provide an implementation with a way to supply its own authorizations, such as to check if the pool contract implementation is in-use or defunct, or if the caller is allowed to use the pool. You could go even further and replace the pool-contracts map with a trait and trait implementation, so the entity that authorizes pools could provide its own authorization mechanism.

To summarize, I don't think this SIP adds anything that can't already be done in Clarity today.

@lgalabru
Copy link
Contributor Author

This is because the value of the contract address loaded from the data space could be the result of a computation running through an arbitrary, unbounded sequence of Clarity contract-calls.

Thank you for your attention on this @jcnelson, this is exactly the kind of design flaws I was looking for - I agree, this proposal is incomplete. Do you see a way we could enforce some constraints somehow ?

To summarize, I don't think this SIP adds anything that can't already be done in Clarity today.

I don't think this is true. With this proposal, you could write a proxy contract making a SIP10 contract upgradable - impossible with the constraints in place today.
To be clear, I'm not attached to this specific proposal, I'm looking for a way to relax the right constraints to make contracts upgradable.

@jcnelson
Copy link
Contributor

I don't think this is true. With this proposal, you could write a proxy contract making a SIP10 contract upgradable - impossible with the constraints in place today.

Allow me to demonstrate:

sip010-token-1:

(impl-trait 'SP1V4XDF7WYFQ829GWZ0D3BJ41E2R16X9JRK945AR.sip010.sip-010-trait)

;; Transfer from the caller to a new principal
(define-public (transfer (amount uint) (sender principal) (receiver principal) (memo (optional (buff 34))))
    (ok true))

;; the human readable name of the token
(define-public (get-name)
    (ok "token-1"))

;; the ticker symbol, or empty if none
(define-public (get-symbol)
    (ok "TKN1"))

;; the number of decimals used, e.g. 6 would mean 1_000_000 represents 1 token
(define-public (get-decimals)
    (ok u100))

;; the balance of the passed principal
(define-public (get-balance (user principal))
    (ok u0))

;; the current total supply (which does not need to be a constant)
(define-public (get-total-supply)
    (ok u0))

;; an optional URI that represents metadata of this token
(define-public (get-token-uri)
    (ok none))

sip010-token-2:

(impl-trait 'SP1V4XDF7WYFQ829GWZ0D3BJ41E2R16X9JRK945AR.sip010.sip-010-trait)

;; Transfer from the caller to a new principal
(define-public (transfer (amount uint) (sender principal) (receiver principal) (memo (optional (buff 34))))
    (ok false))

;; the human readable name of the token
(define-public (get-name)
    (ok "token-2"))

;; the ticker symbol, or empty if none
(define-public (get-symbol)
    (ok "TKN2"))

;; the number of decimals used, e.g. 6 would mean 1_000_000 represents 1 token
(define-public (get-decimals)
    (ok u101))

;; the balance of the passed principal
(define-public (get-balance (user principal))
    (ok u0))

;; the current total supply (which does not need to be a constant)
(define-public (get-total-supply)
    (ok u1))

;; an optional URI that represents metadata of this token
(define-public (get-token-uri)
    (ok none))

proxy:

(use-trait sip010 'SP1V4XDF7WYFQ829GWZ0D3BJ41E2R16X9JRK945AR.sip010.sip-010-trait)
(define-data-var token-contract principal 'SP1V4XDF7WYFQ829GWZ0D3BJ41E2R16X9JRK945AR.sip010-token-1)

(define-private (check-authorized-contract (sip010-contract <sip010>))
    (is-eq (contract-of sip010-contract) (var-get token-contract)))

;; the client calls this via the RPC endpoint to get the right contract address
(define-read-only (get-authorized-contract)
   (var-get token-contract))
   
(define-constant ERR_UNAUTHORIZED u1000)

;; Transfer from the caller to a new principal
(define-public (proxy-transfer (sip010-contract <sip010>) (amount uint) (sender principal) (receiver principal) (memo (optional (buff 34))))
    (if (check-authorized-contract sip010-contract)
        (contract-call? sip010-contract transfer amount sender receiver memo)
        (err ERR_UNAUTHORIZED)))

;; the human readable name of the token
(define-public (proxy-get-name (sip010-contract <sip010>))
    (if (check-authorized-contract sip010-contract)
        (contract-call? sip010-contract get-name)
        (err ERR_UNAUTHORIZED)))

;; the ticker symbol, or empty if none
(define-public (proxy-get-symbol (sip010-contract <sip010>))
    (if (check-authorized-contract sip010-contract)
        (contract-call? sip010-contract get-symbol)
        (err ERR_UNAUTHORIZED)))

;; the number of decimals used, e.g. 6 would mean 1_000_000 represents 1 token
(define-public (proxy-get-decimals (sip010-contract <sip010>))
    (if (check-authorized-contract sip010-contract)
        (contract-call? sip010-contract get-decimals)
        (err ERR_UNAUTHORIZED))) 

;; the balance of the passed principal
(define-public (proxy-get-balance (sip010-contract <sip010>) (user principal))
    (if (check-authorized-contract sip010-contract)
        (contract-call? sip010-contract get-balance user)
        (err ERR_UNAUTHORIZED)))

;; the current total supply (which does not need to be a constant)
(define-public (proxy-get-total-supply (sip010-contract <sip010>))
    (if (check-authorized-contract sip010-contract)
        (contract-call? sip010-contract get-total-supply)
        (err ERR_UNAUTHORIZED)))

;; an optional URI that represents metadata of this token
(define-public (proxy-get-token-uri (sip010-contract <sip010>))
    (if (check-authorized-contract sip010-contract)
        (contract-call? sip010-contract get-token-uri)
        (err ERR_UNAUTHORIZED)))

In your proposal, you're already requiring the client to generate a Merkle tree over the contracts which will be called and provide the Merkle root. In my counterexample, I'm requiring the contract to call the proxy--equivalent SIP010 methods with a trait concretization, which they can learn via the get-authorized-contract read-only function via an RPC call.

@lgalabru
Copy link
Contributor Author

lgalabru commented Mar 27, 2024

You implemented a contract proxying contract-calls to a SIP10 contract, however this is not an upgradable token.
Your proxy is not conforming to the SIP10 interface, which is something you could do with this SIP.

@jcnelson
Copy link
Contributor

jcnelson commented Mar 27, 2024

You implemented a contract proxying contract-calls to a SIP10 contract, however this is not an upgradable token.

Yes it is -- just change the token-contract data var. You can replace .token-1 with .token-2 this way, and clients that use the proxy will be none the wiser.

Your proxy is not conforming to the SIP10 interface, which is something you could do with this SIP.

Your proposal doesn't conform either, because the caller has to provide a Merkle tree of the sequence of called contracts. Just because it isn't in the Clarity code body doesn't mean that existing SIP010 wallets can use an implementation of this proposal ;)

@lgalabru
Copy link
Contributor Author

lgalabru commented Mar 27, 2024

Yes it is.

It is proxying contract calls, but your proposal is not an upgradable contract, in the sense that you would not be able to pass your "proxy" contract to a contract taking a SIP10 contract-id as an argument.

Your proposal doesn't conform either

It does. The caller is providing a merkle root that will be evaluated as a post condition, not as a way to get the contracts to execute.

@jcnelson
Copy link
Contributor

It is proxying contract calls, but your proposal is not an upgradable contract, in the sense that you would not be able to pass your "proxy" contract to a contract taking a SIP10 contract-id as an argument.

Why is it necessary that the proxy contract conforms to SIP010? Your proposal doesn't conform to SIP010 token either, since your proposal requires the caller to supply mandatory extra information (as a post-condition) that is not described in SIP010.

It does. The caller is providing a merkle root that will be evaluated as a post condition, not as a way to get the contracts to execute.

The post-condition is mandatory information. Like I said earlier, just because it isn't in the Clarity code body doesn't mean that existing SIP010 wallets can use an implementation of this proposal ;)

@jcnelson
Copy link
Contributor

jcnelson commented Mar 27, 2024

To be more blunt, I'm steadfastly against adding unchecked dynamic dispatch to Clarity, since that would break one of the core promises the language makes to smart contract developers -- that contracts are decidable. This proposal could work if the Merkle tree root of the reachable contracts was a mandatory post-condition, whereby a transaction that performed dynamic dispatch was required to provide the Merkle root of reachable contracts in order to be evaluated (and, presumably, a Merkle proof that the contract was authorized by the sender for each called contract). I'm saying that if we're going to require this, then it's no better than the existing trait system.

@lgalabru
Copy link
Contributor Author

Thanks, that's useful.

This proposal could work if the Merkle tree root of the reachable contracts was a mandatory post-condition, whereby a transaction that performed dynamic dispatch was required to provide the Merkle root of reachable contracts in order to be evaluated (and, presumably, a Merkle proof that the contract was authorized by the sender for each called contract).

Unless I'm missing something, this would be our common grounds, and what this SIP is proposing, except for the Merkle proof part. You're proposing "reachable contracts", and the proposal in this SIP is a bit more restrictive - something we should follow-up with if there is an alignment on this spec.

I'm saying that if we're going to require this, then it's no better than the existing trait system.

This would be the point of divergence, and what I'm trying to illustrate with the SIP10 exercise.
I'm using the SIP10 interface but that would be true for any other design (oracle, amm, etc).
Let's dive in a little deeper.

If we wanted to create an upgradable token contract, we would probably be splitting the storage and the logic in two different contracts, to avoid liquidity migrations in case of disaster in the logic.
So we would end up with a total of 3 contracts - something along the lines:

The Storage:

;; token-storage.clar

;; Storage Trait
(define-trait token-storage-trait
  (
    (mint (...) (response uint uint))

    (burn (...) (response uint uint))

    (transfer (...) (response uint uint))

    (get-balance (...) (response uint uint))

    (get-total-supply (...) (response uint uint))
  )
)

;; Token storage
(define-fungible-token ...)

;; Implement token storage trait
(define-public mint ...)

(define-public burn ...)

(define-public transfer ...)

(define-public get-balance ...)

(define-public get-total-supply ...)

The Logic, which we'd like to be able to update:

;; token-v1.clar

;; Logic Trait
(define-trait token-storage-trait
  (
    (transfer (<token-storage-trait> uint uint principal (optional (buff 34))) (response uint uint))

    (get-balance (<token-storage-trait> principal) (response uint uint))

    (get-total-supply (<token-storage-trait>) (response uint uint))
  )
)

;; Transfer from the caller to a new principal
(define-public (transfer (token-storage <token-storage-trait>) (amount uint) (sender principal) (receiver principal) (memo (optional (buff 34))))
    ;; Special logic
    (contract-call? token-storage transfer amount sender receiver))

;; the balance of the passed principal
(define-public (get-balance (token-storage <token-storage-trait>) (user principal))
    ;; Special logic
    (contract-call? token-storage get-balance receiver))

;; the current total supply (which does not need to be a constant)
(define-public (get-total-supply (token-storage <token-storage-trait>))
    ;; Special logic
    (contract-call? token-storage get-total-supply))

The proxy, or contract-id that must be something that downstream clients can commit to. Say, the contract-id of USDC, for example.

;; token-contract-id.clar

;; Trait
(impl-trait sip10)

(define-public (transfer (amount uint) (sender principal) (receiver principal) (memo (optional (buff 34))))
    (ok false))

;; the human readable name of the token
(define-public (get-name)
    (ok "token-2"))

;; the ticker symbol, or empty if none
(define-public (get-symbol)
    (ok "TKN2"))

;; the number of decimals used, e.g. 6 would mean 1_000_000 represents 1 token
(define-public (get-decimals)
    (ok u101))

;; the balance of the passed principal
(define-public (get-balance (user principal))
    (ok u0))

;; the current total supply (which does not need to be a constant)
(define-public (get-total-supply)
    (ok u1))

;; an optional URI that represents metadata of this token
(define-public (get-token-uri)
    (ok none))

This last contract is our bottleneck. If we want tokens to be standardized, tokens need to conform to a shared interface and the "internal kitchen" needs to be abstracted. In the present case, we would need to pass the contract ids of both the Logic and the Storage contracts.
With our current approach, either all the protocols use non-upgradeable tokens, or they use upgradeable tokens, pulling the same internal traits, which belongs to the realm of the impossible - tokens have their own constraints and specification - some are centralized, some are bridged, etc.

In this new approach, developers are getting more freedom with their design space, and users would still be able to enforce the decidability through post conditions.

@jcnelson
Copy link
Contributor

jcnelson commented Mar 28, 2024

This last contract is our bottleneck. If we want tokens to be standardized, tokens need to conform to a shared interface and the "internal kitchen" needs to be abstracted. In the present case, we would need to pass the contract ids of both the Logic and the Storage contracts.
In this new approach, developers are getting more freedom with their design space, and users would still be able to enforce the decidability through post conditions.

Your proposal nevertheless requires users to identify the sequence of "internal kitchen" Logic and Storage contract functions that will be invoked; it just moves this outside of Clarity. The fact that it is required means that this proposal does not preserve SIP-010 compatibility (or compatibility with any SIP for that matter) -- it requires wallets to do more things than SIP-010 indicates in order to produce a valid transaction. That's my interpretation of this line in this SIP:

All the branches of this tree are collected, hashed and added in their
order of execution in a Merkle Tree. The Merkle Root constitute the
Execution Plan Commitment that users can incorporate in their transactions
post conditions.

To update my understanding from earlier, the user does not need to provide Merkle proofs. Instead, the VM rebuilds the Merkle tree as the contract executes, and verifies that the root hash matches the execution plan post-condition. Is that understanding correct?

If so, then this is actually more brittle than the trait system today -- users must commit to a specific execution path of the contract, whereas the current system permits any execution since all paths are a priori type-checked and thus safe to follow. Specifically, a contract-call transaction can unintentionally abort (or may even be impossible to generate) if the wallet cannot predict which branch will be taken in advance. This is something that could easily happen if a contract's invocation was dependent on data that cannot be known in advance, such as VRF state or Bitcoin block headers.

If we want tokens to be standardized, tokens need to conform to a shared interface and the "internal kitchen" needs to be abstracted. In the present case, we would need to pass the contract ids of both the Logic and the Storage contracts.

This is readily addressed with the trait system today. You could write a SIP for a fungible token standard where each function had arguments for trait implementations of Logic and Storage. The problem here isn't Clarity; the problem is that SIP-010 does not make any provision for this. However, this does not stop you or anyone from authoring a new fungible token SIP that addressed this.


I can understand the desire to preserve the SIP-010 interface, but it is impossible to do this without severely and systemically crippling Clarity's safety features (which is something that I and others strongly oppose). Moreover, this proposal does not succeed in preserving SIP-010 compatibility, even if execution path commitments were mandatory. The fact that the caller must supply an execution path commitment means that you are already requiring wallets to perform a type-check pass that would normally be handled by the trait system -- a non-trivial behavior that SIP-010 does not specify. This in turn makes me question why they aren't just using the trait system to begin with, especially since it's already live.

I'm starting to feel like a broken record here, but I'll say it again: just because the execution plan commitment isn't in the Clarity code body doesn't mean that existing SIP-010 wallets can use an implementation of this proposal. If they took no action, and if execution plan commitments were mandatory, then they'd generate invalid transactions. So, compatibility is unobtainable. Our three choices are:

  • Keep Clarity the way it is, and publish a new SIP for fungible tokens that accepted trait implementations for Logic and Storage.
  • Implement this proposal, and make execution path commitments mandatory. Then, SIP-010 wallets would all need to be patched to compute execution path commitments for their transactions.
  • Implement this proposal as-is, with optional execution path commitments. Then, Clarity loses a substantial amount of its safety properties and becomes undecidable, thereby forcing users to take more risks when using Stacks than they otherwise would today.

The first option has the most appeal to me, because it is the least disruptive solution to the problem this proposal is trying to solve.

@lgalabru
Copy link
Contributor Author

lgalabru commented Mar 28, 2024

Your proposal nevertheless requires users to identify the sequence of "internal kitchen" Logic and Storage contract functions that will be invoked; it just moves this outside of Clarity.

I'm with you so far, with one nuance: it moves decidability from the Clarity Language specification, to the Clarity VM specification. Which seems fine, non re-entrancy is also enforced at the VM level, not at the language level.

The fact that it is required means that this proposal does not preserve SIP-010 compatibility (or compatibility with any SIP for that matter) -- it requires wallets to do more things than SIP-010 indicates in order to produce a valid transaction.

That I disagree with. SIP-10 is all about Clarity and it's a Clarity interface commitment, period. It is not specifying the contract usage, the wire format of the transactions interacting with SIP10 implementations, etc - these details are abstracted and for good reasons. SIP10 is just assuming backward compatiblity - which this proposal can achieve - something that this SIP should be describing that I can unpack here.

I think that the execution plan commitment would not be required for every single transaction.

(define-public (increment-counter) (ok u1))

It would be mandatory for any transaction involving contract-call dynamic dispatch v2. On contract deployments, we would be tainting methods using dd v2, the same way we're tainting read / write behaviors today.
With dynamic dispatch v1, we already have the decidability enforced at the language level, so enforcing the decidability at the VM level is redundant.

Back to our Sip10 scenario:
A token alone is not very useful, it's just a low level primitive, so let's bubble up and consider a simple, non-upgradable swap-ddv1-sip10 contract developped by Bob, exchanging sip10 tokens.
We have 3 tokens, 2 non upgradables (tkn-a, tkn-b), 1 upgradable tkn-z - the 3 tokens are committing to the same sip10 interface shape.
Alice using swap-ddv1-sip10 with tkn-a, tkn-b would not require an execution-plan commitment post-condition.
Alice using swap-ddv1-sip10 with tkn-a, tkn-z would require an execution-plan commitment post-condition.

As clearly illustrated here, from an ecosystem point of view, the interfaces stayed the same, the transition is seamless and swap-ddv1-sip10 did not require a re-deployment - breaking downstream components.
Users are informed that there is a level of dynamism and securing their transaction.

Pushing the experiment further, with an upgradable swap-ddv2-sip10 contract. In this case
Alice using swap-ddv2-sip10 with tkn-a, tkn-b would require an execution-plan commitment post-condition.
Alice using swap-ddv2-sip10 with tkn-a, tkn-z would require an execution-plan commitment post-condition.

No surprise here, the commitment is required in any case, coming from swap-ddv2-sip10 being tainted.

If we unpack this 2 scenarios with our current design, the complexity that Bob needs to deal becomes quadratic: the swap-ddv1-sip10 needs to implement 4 methods instead of one:

  • swap(upgradable, non upgradable)
  • swap(non upgradable, upgradable)
  • swap(non upgradable, non upgradable)
  • swap(upgradable, upgradable)

And this is assuming that all the developers were able to settle on the fact that tokens should all have one logic and one storage layer with a very specific trait. A new kind of token (let say a bridged token, that would probably have a very specific shape)? We can square the number of methods required + redeploy a new contract (and breaking all the downstream dependencies).
Note also that in this case, the wallet would have to retrieve the right version of the contracts to set in the high level contract call arguments - whatever the patterns, there is some extra work required when dealing with upgradeable contracts.

To update my understanding from earlier [...] If so, then this is actually more brittle than the trait system today

I'm curious, at what point decidability became brittle? If anything, this proposal is achieving a level of decidability that we could not reach at the language level.
As a user interacting with a contract invoking contract-a or contract-b depending on the value of a VRF, I'd feel much, much safer if I can enforce that I only want to get my transaction executed if contract-a is being called.
This would probably not hold for gambling patterns, I guess.

I'm starting to feel like a broken record here, but I'll say it again: just because the execution plan commitment isn't in the Clarity code body doesn't mean that existing SIP-010 wallets can use an implementation of this proposal. If they took no action

Sure, point taken. Concretely speaking, we are talking about wallets corretcly handling a new major feature coming with a Stacks network upgrade. It would require coordination, like any other network upgrade. So we would be breaking downstream off-chain components, but not the on-chain components, which seems like the right approach to me.

It is impossible to do this without severely and systemically crippling Clarity's safety features

I would love to focus the debate on this. I still don't understand how this proposal is "crippling Clarity's safety features".

Our three choices are

  • Keep Clarity the way it is, and publish a new SIP for fungible tokens that accepted trait implementations for Logic and Storage.
  • Implement this proposal, and make execution path commitments mandatory. Then, SIP-010 wallets would all need to be patched to compute execution path commitments for their transactions.
  • Implement this proposal as-is, with optional execution path commitments. Then, Clarity loses a substantial amount of its safety properties and becomes undecidable, thereby forcing users to take more risks when using Stacks than they otherwise would today.

I suppose adding that third choice was sarcastic.
We have 2 choices really but let's try to start with a common ground:
Stacks contracts are not upgradable, and I'm really surprised to see that you're trying to convince me and demonstrate that they are (cf the first debate about versioning we had 5 years ago).
All the choices we've made were designed to forbid upgradability. Non upgradable contracts was a feature, not an accident. You can approach upgradability, but you can't achieve it.
The 2 choices are:

  • Stand on our grands, we still believe that upgradability is not desirable feature and that clarity developers must be able to produce code without bugs.
  • Revisit our stance. Explore this SIP further, tweak it, improve it to the point we can convince ourselves that we're not compromising first principles.

@jcnelson
Copy link
Contributor

I'm with you so far, with one nuance: it moves decidability from the Clarity Language specification, to the Clarity VM specification.

The unstated caveat here is that execution path commitments must be mandatory to preserve decidability. Your proposal currently does not state this, btw.

That I disagree with. SIP-10 is all about Clarity and it's a Clarity interface commitment, period. It is not specifying the contract usage, the wire format of the transactions interacting with SIP10 implementations, etc - these details are abstracted and for good reasons. SIP10 is just assuming backward compatiblity - which this proposal can achieve - something that this SIP should be describing that I can unpack here.

This proposal absolutely does not maintain backwards compatibility. It provides the illusion of backwards compatibility by "sweeping under the rug" the need for wallets to produce this new piece of data to produce a valid transaction. Every single wallet will be affected by this proposal, as you point out later on. Furthermore, your example illustrates the preconditions for a wallet to produce an execution path commitment -- something that no wallet today can do.

You have said all of this yourself.

It is impossible to do this without severely and systemically crippling Clarity's safety features
I would love to focus the debate on this. I still don't understand how this proposal is "crippling Clarity's safety features".

Removing decidability (which this proposal does, as written) would cripple Clarity's safety features. I'm dead serious. Language-level decidability is a feature that I see no reason to remove, especially since the problem your proposal is trying to solve can already be solved via the existing language features.

Stacks contracts are not upgradable, and I'm really surprised to see that you're trying to convince me and demonstrate that they are

I have demonstrated that you can alter the business logic that runs behind a fixed API through trait concretization. However, you seem to have an aversion to the principle of requiring the contract caller to supply a trait concretization. If the contract caller provides trait concretizations, then it is possible today to "upgrade" contracts by keeping its interface constant but changing its implementation:

  • The contract author specifies their contract's business logic interface as one or more traits (e.g. Logic, Storage).
  • The contract author publishes a proxy contract which routes contract-calls to an author-determined concretization of those traits.
  • The proxy contract provides the programmatic means for wallets to query the currently-supported concretizations to use in their contract-call transactions ((get-authorized-contract)), and for the contract author to change which concretizations are valid ((var-set token-contract ...), which can easily be wrapped in a public function which checks that tx-sender and contract-caller are authorized to change it).

So, the wallet's steps for using an upgradeable contract are:

  1. Query the proxy contract for the latest trait concretizations
  2. Provide those concretizations as arguments in the contract-call transaction
  3. Sign and broadcast the contract-call transaction

And, the contract admin's steps to upgrading the contract are:

  1. Deploy a new implementation that implements the business logic interface's trait(s)
  2. Update the addresses of these implementations in the proxy contract

As you saw in my example, the author of a SIP-010 token can create multiple versions of the token, and require users of the token to interact with a specific version via the proxy contract.

I'm sorry if that does not match your vision of how the system ought to work, but it works today and is safer. The only thing that's missing here is a SIP for describing a SIP-010 proxy contract.

@hstove
Copy link
Contributor

hstove commented Mar 28, 2024

@jcnelson the main point I'd want to speak to in your examples of "how this is doable today" is that a huge goal I have is to remove the need for authorization checks when calling traits. I'm not saying you never need authorizations, just that in some cases it would be better if it wasn't mandated. Regarding the AMM example, I don't think it's good that to have an AMM with pool contracts you have to have authorization checks.

Ultimately, though, I care a lot more about contract factories (or the ability to verify a contract's source) as a means toward solving those goals. I'm still not sold on the need for dynamic dispatch.

If you look at something like Uniswap, one of its best features is that you can swap any token for any token without anyone telling you whether it's ok or not. Additionally, you can swap any token without needing to worry about some centralized entity having added a bogus pool contract.

Regarding many of your other concerns, I would mainly agree that this SIP is currently too undefined, especially as it relates to very vague handling of things like calling a stored principal when you don't even know if it conforms to a trait. I think there are ways we could approach that and keep decidability, assuming we have a solid mechanism of whitelisting execution plans and blocking anything that doesn't have one.

At the very least, I don't think we should allow calling a stored principal. One way to approach this is to allow storing data with trait types, with no mechanism to store data with a regular principal:

(use-trait .contract pool-trait)

(define-map pools uint <pool-trait>)

(contract-call? (unwrap-panic (map-get? pools uint)) some-func)

@jcnelson
Copy link
Contributor

@jcnelson the main point I'd want to speak to in your examples of "how this is doable today" is that a huge goal I have is to remove the need for authorization checks when calling traits. I'm not saying you never need authorizations, just that in some cases it would be better if it wasn't mandated. Regarding the AMM example, I don't think it's good that to have an AMM with pool contracts you have to have authorization checks.

I think your contract-hash keyword concept would be straightforward to specify and implement. It might even be able to ship with Nakamoto, if time permits.

@MarvinJanssen
Copy link
Collaborator

MarvinJanssen commented Apr 2, 2024

Based on our conversations and reading the proposal, am I correct in that your aim is to achieve the following:

  1. Create a smart contract that conforms to some standard / trait (like SIP010.)
  2. Have the ability to upgrade the contract's logic without affecting its storage.
  3. Keep the contract principal the same after an upgrade happens.

As has already been mentioned in the thread, a lot can be done already with current language features. I think it really depends on the desired effect. The easiest solution is obviously to create a wrapper or proxy contract that requires a trait, but that alters the function signature such that a new standard is required. I can think of a few other patterns that go from less to more invasive.

Upgradable trait / SIP

If we drop aim number 3 (keeping the contract principal the same) then I can envision quite a simple SIP that can introduce contract upgradability. In fact, it is a pattern that we have been following ourselves for some projects. The crux is to have one core contract that accepts a trait, preceded by concrete implementations. The core contract can then report on which implementation to use. The SIP will need to define a way for wallets to discover what the current implementation is. In the example below, a special error err-do-upgrade is introduced. When a wallet encounters it, it should call get-current-implementation to find the newest version of the contract.

token-core.clar:

(define-constant err-do-upgrade (err u100))

;; This would be a circular reference, but it is just for illustration purposes.
;; In the real world you would deploy both contracts and then update the
;; data-var.
(define-data-var current-implementation .token-v1)

(define-fungible-token my-token)

(define-read-only (get-current-implementation)
	(ok (var-get current-implementation))
)

(define-read-only (assert-implementation-caller)
	(ok (asserts! (is-eq contract-caller (var-get current-implementation))) err-do-upgrade)
)

(define-public (upgrade (new-implementation principal))
	(begin
		;; (asserts! ...)
		;; whatever guard you have to protect the upgrade: DAO, multisig, something else.
		(ok (var-set current-implementation new-implementation))
	)
)

(define-public (transfer (amount uint) (sender principal) (recipient principal))
	(begin
		(try! (assert-implementation-caller))
		(ft-transfer? my-token amount sender recipient)
	)
)

;; (define-public (mint)....)
;; and so on

And token-v1.clar:

(impl-trait 'SP3FBR2AGK5H9QBDH3EEN6DF8EK8JY7RX8QJ5SVTE.sip-010-trait-ft-standard.sip-010-trait)

(define-constant err-unauthorised (err u500))

(define-read-only (get-current-implementation)
	(contract-call? .token-core get-current-implementation)
)

(define-public (transfer (amount uint) (sender principal) (recipient principal) (memo (optional (buff 34))))
	(begin
		(asserts! (or (is-eq sender contract-caller) (is-eq sender tx-sender)) err-unauthorised)
		(match memo to-print (print to-print) 0x)
		(contract-call? .token-core transfer amount sender recipient)
	)
)

;; and so on...

To upgrade, deploy a new token-v2 and update the current-implementation data-var in token-core.

Dynamic implementation trait / SIP

We can inverse the above and have a stub contract that forwards its calls to an actual implementation. There is a bit more complexity but you can use it to achieve a "delegate call" style pattern by forwarding tx-sender. We have also used this pattern successfully in a project recently. The wallet only needs to interact with the proxy but will have to switch once a new implementation is authorised. It is the same as what Jude suggested, with the addition of a proxy/stub that pre-fills the trait reference. The basic pattern is such:

token-holder.clar:

(use-trait token-implementation .token-implementation-trait.token-implementation-trait)

(define-constant err-unauthorised (err u100))

;; Can also be a map obviously
(define-data-var authorised-caller .token-proxy)

(define-read-only (get-authorised-caller)
	(ok (var-get authorised-caller))
)

(define-read-only (assert-authorised-caller)
	(ok (asserts! (is-eq contract-caller (var-get authorised-caller))) err-unauthorised)
)

(define-public (set-authorised-caller (new-caller principal))
	(begin
		;; (asserts! ...)
		;; whatever guard you have to protect the upgrade: DAO, multisig, something else.
		(ok (var-set authorised-caller new-caller))
	)
)

(define-public (transfer-out (token-implementation <token-implementation-trait>) (amount uint) (recipient principal) (memo (optional (buff 34))))
	(begin
		(try! (assert-authorised-caller))
		(as-contract (contract-call? token-implementation transfer-out amount recipient memo))
	)
)

token-implementation.clar:

(define-public (transfer-out (amount uint) (recipient principal) (memo (optional (buff 34))))
	(begin
		;; tx-sender will be .token-holder here
		(match memo to-print
			(stx-transfer-memo? amount tx-sender recipient memo)
			(stx-transfer? amount tx-sender recipient)
		)
	)
)

token-proxy.clar

(define-public transfer-out (amount uint) (recipient principal) (memo (optional (buff 34)))
	(begin
		;; (asserts! ...)
		;; whatever guard is required
		(contract-call? .token-holder transfer-out .token-implementation amount recipient memo)
	)
)

To upgrade, deploy a new implementation and allow it in token-holder. Deploy a new proxy to mask the trait reference parameter.

Contract principal must stay the same

If aim 3 is a must, then things become harder. Essentially, you would want to provide a trait reference without actually specifying it as a function parameter. I can imagine perhaps that a type of post condition is introduced that admits such a reference, and a new magic constant in Clarity that provides for it. But I assume that makes the post condition mandatory and in the end it is just kicking the can down the road. The wallet still needs to be aware of it and construct a post condition with the trait, lest the call fails. Furthermore, it will introduce complications for contracts wanting to call into such a contract, requiring a new kind of contract-call? that can set the special post condition.

Authorisation checks side-note

Regarding the AMM example, I don't think it's good that to have an AMM with pool contracts you have to have authorization checks.

I consider this a flaw in the SIP010 standard, not the AMMs. They need to protect themselves from problematic SIP010 tokens. The SIP010 standard does not specify the required guard for the transfer function. Most SIP010 implementations use simply (is-eq sender tx-sender), which forces contracts to use as-contract in order to move tokens they own; e.g. you will see:

(as-contract (contract-call? sip010-trait transfer amount tx-sender recipient memo))

If all SIP010 tokens instead had the guard (or (is-eq sender tx-sender) (is-eq sender contract-caller)) then contracts would not have to resort to as-contract. A topic for a different SIP, but I think if contracts could specify their own post conditions when calling into another contract then a lot would change.

@lgalabru lgalabru changed the title SIP: Optimistic Dynamic Dispatch and Execution Plan Commitment SIP: Optimistic Dispatch and Execution Plan Commitment May 17, 2024
@jcnelson
Copy link
Contributor

Some of the people on this thread had a call about this SIP. Summarizing, the following ideas could be their own SIPs:

  • Execution Plan Commitments (EPC). This SIP would propose enabling a contract-call transaction can commit to the sequence of contract-calls (and possibly some of their argument values) that it expects to be executed. Unlike this SIP, an EPC would always be an optional post condition, and no changes to the Clarity language would be required. This is helpful because today, the user does not (and cannot) know the exact execution path their code will take; they can only know the set of all possible execution paths. However, requiring an exact execution path would mitigate the class of exploits which depend on data-dependent branches that are only known at runtime. For example, today a malicious contract could temporarily redirect a contract-call's execution path to code in which the caller's tokens get stolen, if the transaction which performs this temporary redirection gets mined after the user submits their contract-call but before their contract-call gets mined. An EPC would prevent this.

  • Contract Hashes. This is alluded to above, but there could be a SIP which proposes making the hash of a contract available in Clarity, such as through a built-in (contract-hash <principal>) function. This would enable contracts which interact with many trait concretizations to bulk-allow (or bulk-deny) concretizations based on their code bodies, instead of their code deployment addresses.

  • Contract-supplied Post-conditions. This has been discussed elsewhere but is relevant here as well. There could be a SIP to add a built-in Clarity function to supply post-conditions to a code block, including EPCs. The post-conditions themselves would not need to be constant; they could be (stored) data, or even arguments from the calling function. This would allow a smart contract to sandbox the behaviors of trait concretizations they call. For example, a DEX smart contract could use contract-supplied post-conditions to prevent a concretization from touching assets that don't belong to it. With EPCs, a DEX could further require a concretization to only call a specific sequence of contract functions, in a specific order.

  • Trait Concretization Discovery. It is possible to implement upgradable contracts today by requiring the contract-caller to provide a concretization of a trait which defines the contract's upgradable business logic (an example is given above). However, because concretizations must be supplied by the contract-call transaction itself (as opposed to data within a smart contract), any contract function which calls an upgradable contract must take the trait parameters for the upgradeable contract as arguments. This places a burden on wallets to find the right concretizations to use. To facilitate this, there could be a SIP that standardizes the way to discover concretizations for a particular class of traits via a discovery contract. For example, there could be a global singleton discovery contract for SIP-010 business logic, in which SIP-010 authors bind a fixed name to the address of their latest SIP-010 implementations. Wallets calling a contract which requires a SIP-010 concretization for a so-named SIP-010 token would first query this discovery contract to resolve the SIP-010 token name to the address of the concretization.


In addition to the above, there is some concern about how requiring concretizations for traits to be supplied by the contract-caller can make the arity of some contract functions grow unbound. For example, without careful design, a DEX smart contract's swap function might be compelled to take hundreds of trait concretizations (many of which the caller may not even need or care about) in order to correctly call any reachable business logic that uses them. Some research into how other smart contract systems is needed to determine whether or not this is a concern in practice. If the required arity can be kept small (e.g. no more than eight concretizations for practical applications), then no further changes to the Clarity language may be needed. However, if it cannot be kept small, then the question of whether or not Clarity should support dispatch from a stored trait concretization (which this SIP proposes) will need to be revisited.

@andrerserrano
Copy link

I wanted to share more context for why this upgrade is needed from the business perspective.

The Stacks BD Working Group has received consistent feedback that lack of smart contract upgradeability severely limits the types of integrations that can be enabled by our partners. Smart contract upgradability is a requirement for both tier-1 stablecoins, such as USDC and Frax, and cross-chain bridges, such as Axelar.

This line from the SIP seems to summarize it well:

In the context of working with critical partners, especially those operating bridges between different blockchains or managing large amounts of liquidity, contract upgradability becomes a vital feature. This is because errors in these high-stakes environments can propagate across multiple blockchains, leading to extensive financial and operational damage. Thus, having the flexibility to upgrade contracts to address potential issues quickly is a key requirement for maintaining robust and secure blockchain ecosystems.

Bridges and stablecoins are critical for the success of Stacks ecosystem, and by extension sBTC. They help to build liquidity in DeFi apps and improve interoperability with other blockchain ecosystems. Specifically, upgradeable smart contracts are needed for Axelar’s canonical USDC bridge and eventually native USDC issuance itself. Currently, both of these integrations are blocked on this upgrade.

Smart contract upgradeability enables developers to fix bugs, improve functionality, and respond to changes in markets or tech upgrades — all without the need to migrate to a new version of the application (which results in liquidity fragmentation and poor UX). This has become a standard in other blockchain ecosystems with best practices to implement them in a secure way.

As a next step, it would help to do a RICE scoring of the four proposed solutions to this SIP; to assess the level of effort to implement, and determine which of these methods is the most simple and secure way to achieve the requirements.

@jcnelson
Copy link
Contributor

The Stacks BD Working Group has received consistent feedback that lack of smart contract upgradeability severely limits the types of integrations that can be enabled by our partners. Smart contract upgradability is a requirement for both tier-1 stablecoins, such as USDC and Frax, and cross-chain bridges, such as Axelar.

As detailed in the above thread, it is already possible to upgrade contracts in Clarity by way of using a proxy contract and a trait concretization.

@lgalabru
Copy link
Contributor Author

Hi everyone,

I hope you're all doing well. Marvin has conducted some research on contract upgradability that should shed some light on our next steps.

Let's start by establishing some common ground and defining what contract upgradability means for those coming from EVM chains. Whether we like it or not, Solidity has set a standard in terms of expectations, and we want to ensure that we're not misleading our partners with our own definitions.

A contract is considered upgradable if its code can be updated without impacting downstream clients, both off-chain and on-chain. The update can be minor, such as fixing a contract bug, or it can involve augmenting the contract with additional logic or data. With upgradable contracts, developers are prepared for unknown unknowns.

According to this definition, are Clarity contracts upgradable? No. This was debated pre Stacks 2.0, and non-upgradable contracts were a feature and a requirement. Affirming that contracts are upgradable would be admitting that our design is flawed.

Can we leverage dynamic dispatch (a feature unrelated to contract upgradability) and some other hacks to bend the rules and approach upgradability? Yes.

The following repository outlines the steps to achieve an upgradable contract prepared for unknown unknowns without the explosion of traits I anticipated.

I invite core developers to review this contract. It's very informative, and the use of

(define-map data-storage (string-ascii 16) (buff 2048))

as a generic key-value storage (storing uint, tuples, lists, etc) is a stroke of genius. This workaround was missing from my arsenal, and I'm convinced that it could enable the creation of upgradable tokens - truly prepared for unknown unknowns.

Now, back to what motivated this SIP.

We are trying to attract multi-chain partners who are highly scrutinized or have been hacked in the past, and onboarding them with non-upgradable tokens is not an option.

I initially created this spec, thinking that upgradable tokens would not be possible. Marvin has proven me wrong, which now leaves us with three options instead of two (assuming that ignoring the problem is not an option):

  1. [new] Revisit SIP 10 and have developers base their token implementations on the approach outlined in the repository above. A new SIP 10 spec would create standard fragmentation, and the consequences of this are not fully measured.

  2. Adopt Optimistic Dispatch, which will provide backward compatibility and upgradability as a first-class citizen without breaking decidability.

  3. Create a new SIP to address the problem we're trying to solve with a different approach.

Looking forward to your thoughts.

@jcnelson
Copy link
Contributor

jcnelson commented Jun 20, 2024

Let's start by establishing some common ground and defining what contract upgradability means for those coming from EVM chains. Whether we like it or not, Solidity has set a standard in terms of expectations, and we want to ensure that we're not misleading our partners with our own definitions.

A contract is considered upgradable if its code can be updated without impacting downstream clients, both off-chain and on-chain.

According to this definition, are Clarity contracts upgradable? No.

Ethereum contracts are not upgradable either according to this definition, so I reject your premise that Solidity has set some expectations regarding contract upgradability that we are not meeting. You can see for yourself in their developer docs -- EVM contracts cannot be modified in situ, so "upgrading" a contract necessarily involves deploying new contracts at new addresses, and organizing them in such a way that the business logic's flow control is transferred from the old contracts to the new contracts. Furthermore, Stacks supports all of the same upgrade patterns described in that linked document, albeit via client-supplied trait concretizations.

[new] Revisit SIP 10 and have developers base their token implementations on the approach outlined in the repository above. A new SIP 10 spec would create standard fragmentation, and the consequences of this are not fully measured.

People will write contracts that solve their business problems with or without a corresponding SIP, so I'm not too worried about fragmentation. Also, the SIP process makes a provision for SIPs to supersede others, so a future fungible token SIP could render SIP-010 obsolete. This sort of thing happens all the time in standards bodies.

Adopt Optimistic Dispatch, which will provide backward compatibility and upgradability as a first-class citizen without breaking decidability.

As I've said above, I would be okay a new post-condition which commits the Clarity VM to execute a particular sequence of contract-calls, since there's value to this on its own. But, this says nothing about contract upgradability.

Create a new SIP to address the problem we're trying to solve with a different approach.

Since we've established that Stacks contracts are indeed upgradable in the same manner that Ethereum contracts are, I think we need to decide what problem we're actually solving here.

@lgalabru
Copy link
Contributor Author

lgalabru commented Jun 20, 2024

I would be okay a new post-condition which commits the Clarity VM to execute a particular sequence of contract-calls, since there's value to this on its own.

This SIP is bundling 2 ideas: A) Optimistic Dispatch and B) Execution plan commitment. We could implement A) and B), B) without A), but not A) without B).
Are you saying that you're green lighting B) but not A)? If that's the case, I don't think this will be helpful here, meaning we'd be left with:

  1. [new] Revisit SIP 10 and have developers base their token implementations on the approach outlined in the repository above. A new SIP 10 spec would create standard fragmentation, and the consequences of this are not fully measured.

  2. Adopt Optimistic Dispatch, which will provide backward compatibility and upgradability as a first-class citizen without breaking decidability.

  3. Create a new SIP to address the problem we're trying to solve with a different approach.

I left our last call with the belief that there was alignment on the fact that this SIP is not degrading Clarity decidability. An explanation of what might seem like a step back to me or what I might have misunderstood would be very helpful. 🙏

Since we've established that Stacks contracts are indeed upgradable in the same manner that Ethereum contracts are

I don't believe this is entirely accurate. Without Optimistic Dispatch, we're left with an unresolved trait discovery issue. All the upgradable contracts in a contract call chain will provide a contract address along with an additional contract reference pointing to the implementation contract. Wallets and Dapps will need to discover these implementation contracts somehow (a step that this SIP is fixing)

what problem we're actually solving here.

How can we allow a developer to launch a SIP10 token that can be upgraded, without impacting the existing ecosystem of contracts built on top of the SIP10 standard (DEX, bridges, stablecoin, amm, lending, etc)?

@jcnelson
Copy link
Contributor

jcnelson commented Jun 20, 2024

This SIP is bundling 2 ideas: A) Optimistic Dispatch and B) Execution plan commitment. We could implement A) and B), B) without A), but not A) without B).
Are you saying that you're green lighting B) but not A)? If that's the case, I don't think this will be helpful here, meaning we'd be left with:

First, I can't "greenlight" anything when it comes to a breaking consensus change like this. Such changes only take effect if there's sufficient Stacks user support. If someone did the necessary work of writing an adequate SIP, writing and testing the code, and getting that code merged into the blockchain repo, then I'd have no choice to but to follow the SIP process to determine whether or not the code activates.

Second, per the call we had (and per the first bullet point in my write-up of said call), there is value in having an execution plan commitment post-condition by itself. So, I would support an effort to add this post-condition, and would be willing to spend time writing the code and the SIP to add it.

I left our last call with the belief that there was alignment on the fact that this SIP is not degrading Clarity decidability. An explanation of what might seem like a step back to me or what I might have misunderstood would be very helpful. 🙏

This SIP, as written, absolutely degrades Clarity decidability, as we established in both this comment thread and on the call. So, the text of this SIP needs to be substantially revised before I'll update my assessment.

But on the call, we also agreed that even without any changes to Clarity, it's currently not possible to determine in advance what the halting state of a contract-call will be. The best we can do today is enumerate all of the possible halting states, based on reachability analysis from the contract-call. The reason we can't be more precise is because flow control can branch based on data that was not known (and could not have been known) to the transaction sender. This is why I'm supportive of an execution plan commitment, because it would give the transaction sender the ability to abort a transaction that did not reach an intended halting state.

I suspect that USDC et al. would like this new post-condition by itself, since it would allow them to protect themselves from rogue contracts that were reachable from a contract-call transaction. Also, per the call, I suspect they would appreciate a contract-hash Clarity function (point 2 in my write-up), because it would enable them to check trait concretizations against known-good code bodies (instead of specific addresses).

Without Optimistic Dispatch, we're left with an unresolved trait discovery issue.

This is addressed in point 4 of my write-up. We'll need a SIP that describes a trait discovery protocol. I personally find this a lot more appealing than optimistic dispatch because (1) this SIP would not consensus-breaking, and (2) this SIP only concerns a smart contract design pattern. It would be much easier to implement and ratify, and would be much easier to iterate on and experiment with.

Wallets and Dapps will need to discover these implementation contracts somehow (a step that this SIP is fixing)

The SIP's text does not speak to this yet, so I don't yet agree that this SIP fixes this.

How can we allow a developer to launch a SIP10 token that can be upgraded, without impacting the existing ecosystem of contracts built on top of the SIP10 standard (DEX, bridges, stablecoin, amm, lending, etc)?

I suppose that would depend on what you mean by "impacting." Ethereum addresses this problem with the aforementioned upgrade patterns, which Stacks also supports. I've heard no complaints here about Ethereum's approach, but somehow Stacks' approach is inadequate, despite being functionally the same?

We spoke on the call about the potential for Stacks contracts to encounter a "trait explosion" whereby the caller would need to supply an unreasonable amount of trait concretizations in contract-calls in order to effectively implement smart contract upgrades. But at the end of the call, we agreed that you would do some research and find out whether or not a trait explosion would happen in practice. It seems that for SIP-010 tokens, we have an answer by way of @MarvinJanssen's experimental token contracts -- it does not happen, because in the limit the contract author can store encode and decode version-specific state as buffers, thereby implementing forward-compatible data storage that is "truly prepared for unknown unknowns" (your words).

@wileyj
Copy link
Contributor

wileyj commented Jun 21, 2024

@andrerserrano @lgalabru
you've mentioned in this PR a few times how USDC et al requires upgradeable contracts, but it's not clear what that means to them.
Can you follow up with them and share what their understanding is?
Right now, it feels like we're trying to solve a problem where we cannot agree what the problem even is.

Further, after reading through this entire thread - i'm left with the impression that without having to modify the VM, and thanks to the research by @MarvinJanssen , we could use similar contract "upgrades" to EVM. I think it's important that get more info and we all agree on what we're trying to solve, rather than trying to solve what we think the problem is.

@andrerserrano - could you get this info and share it? i'm particularly interested in what makes the EVM approach compatible for them, but not the Clarity approach (considering the examples and other research above).

Basically, I think we all need to understand and agree what we're trying to do here - and the current discussion/proposal make that less than clear.

@lgalabru
Copy link
Contributor Author

lgalabru commented Jun 21, 2024

@wileyj I see how this whole discussion can be confusing.

The debate is no longer on whether or not we can or not implement an upgradable contract in clarity.
Yes we can, Marvin is proving us that this is possible thanks to generic storage maps.

Our problem is that, per our research, the SIP10 trait specification does not allow us to create an upgradable token contract.

  1. Are we introducing a new SIP10 standard that can be upgraded, without introducing a Blockchain Consensus breaking change?

  2. Are we exploring Optimistic Dispatch + Execution Plan Commitment, giving us the ability to transform the SIP10 standard to an upgradable SIP10 standard.

  3. It's large design space is large, is there another solution?

@wileyj
Copy link
Contributor

wileyj commented Jun 22, 2024

  1. Are we introducing a new SIP10 standard that can be upgraded, without introducing a Blockchain Consensus breaking change?
  2. Are we exploring Optimistic Dispatch + Execution Plan Commitment, giving us the ability to transform the SIP10 standard to an upgradable SIP10 standard.

i think my questions are more geared towards these points - why are the suggestions in this thread to solve for this inadequate? is there a use-case that i may have missed (it is a long thread, i may have missed something).

I get the sense that this SIP is in reaction to something external, and i think we'd all benefit from learning more about that.

@obycode
Copy link
Contributor

obycode commented Jun 24, 2024

I have a concern about the real-world security of the execution plan commitment, because in practice it will be so difficult to understand, that users will just ignore it and blindly agree to it. I hate to add another option to consider, but the thing that comes to my mind for me is, instead of adding an execution plan commitment, we could instead add data pre-/post-conditions, where a user can specify constraints on the expected value of stored data (data-var or map) before and after a transaction's execution. This seems easier to understand, as a user, than specifying the execution plan, but you can gain the same guarantees. This could still enable the desired functionality, of being able to store a trait and then safely execute it.

Less importantly, but an added bonus is that this would likely be simpler to represent in a transaction and simpler to implement.

@renashah
Copy link

renashah commented Jun 25, 2024

Hey all! I wanted to briefly chime in that this is an essential building block to establishing DeFi with the Stacks ecosystem. I've been scoping this out with a stablecoin provider as part of the BD WG goals for a reputable stablecoin issued to the ecosystem. A provider is narrowing down on upgradeability for Stacks because of the need for proxy contracts like ERC 20.

Specifically, our missing requirements in current SIP 010 standards are:

  • Approve/Spending pattern not part of the SIP10 token standard (decrease/increase spending amount)
  • Proxy contracts

The proxy contacts (upgradability) are required by the provider for the following items:

  1. Upgrade the contract - change the implementation contract address
  2. Create roles function - create the admin function
  3. Update read functions - anyone can call (implementation, admin, and pending functions)
  4. Update write functions - changeAdminRole, accessAdminRole, UpgradeTo (Change the implementation contract), upgradeToAndCall (Change the implementation contract and call a function on the new implementation contract)

This will be a requirement for a stablecoin provider and all the external bridge providers. I'm happy to chat more here - appreciate it.

@jcnelson
Copy link
Contributor

Hi @renashah, thank you for providing this context!

At a high level, it looks like the main barrier for this stablecoin provider is that the SIP-010 token standard is insufficient. If so, then perhaps all that is really needed here is a new fungible token standard. Is my understanding correct?

As discussed at length above, Stacks supports contract upgrades through proxy contracts, just like EVM. Stacks simply has a different (safer) mechanism for doing so, however, which may confuse developers coming from an EVM world.

The rationale at the time of SIP-010's ratification for omitting approval/spending like in ERC-20 was because Stacks transaction post-conditions made them unnecessary. The discussion on this topic begins here, in the original SIP-010 pull request. However, this could be revised in a superseding fungible token SIP if we have users who nevertheless require the approval/spending pattern.

@andrerserrano
Copy link

At a high level, it looks like the main barrier for this stablecoin provider is that the SIP-010 token standard is insufficient. If so, then perhaps all that is really needed here is a new fungible token standard. Is my understanding correct?

I think our design goal should be to upgrade SIP10, rather than create a new fungible token standard. Consider all the different stakeholders this would impact: exchanges, custodians, wallets, app developers — all would need to migrate to this new standard. It would seem like this presents a significant risk of fragmenting the ecosystem around two incompatible standards and creating large amounts of operational overhead.

Is it possible to achieve the requirements Rena described by upgrading the SIP10 standard and without the complexity of a new standard?

@jcnelson
Copy link
Contributor

I think our design goal should be to upgrade SIP10, rather than create a new fungible token standard. Consider all the different stakeholders this would impact: exchanges, custodians, wallets, app developers — all would need to migrate to this new standard. It would seem like this presents a significant risk of fragmenting the ecosystem around two incompatible standards and creating large amounts of operational overhead.

That is an unrealizable goal, I'm afraid. A blockchain which permits past transactions (such as instantiations of SIP-010 contracts) to be retroactively modified is no longer a blockchain. But even putting ontology aside, we should all be vehemently against such a feature, because it would have the undesirable consequences of (1) creating new moral hazards for smart contract authors, and (2) shifting technical and possibly even legal responsibility for smart contracts' correctness onto the core developers and SIP officers. If it were permissible to retroactively change any existing smart contract (even if only via a SIP), then it removes a strong motivation for developers to ship high-quality code because they and their rugged users could instead compel the core developers and SIP officers to fix it for them (possibly via a court order, no less). Furthermore, making such a change to the codebase would require rewriting substantial parts of the blockchain implementation, and could easily take a year.

It's better for everyone if you instead worked on a new token standard SIP. Ideally, it would offer a means of providing backwards compatibility with existing SIP-010 tokens. There is precedent for this -- for example, ERC-223 bills itself as an upgraded ERC-20, and offers ERC-7417 as a means to migrate an ERC-20 token to ERC-223 and back.

Copy link
Contributor

@314159265359879 314159265359879 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of miner improvements to spelling/clarity while I read through.


# **License and Copyright**

This SIP is made available under the terms of the Creative Commons CC0 1.0 Universal license, available at https://creativecommons.org/publicdomain/zero/1.0/ 
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

End with a period here.


During the execution of a contract-call transaction, the Clarity VM builds a tree of the different contracts and public methods invoked.

All the branches of this tree are collected, hashed and added in their order of execution in a Merkle Tree. The Merkle Root constitute the Execution Plan Commitment that users can incorporate in their transactions post conditions.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second sentence, constitutes and post-conditions:

The Merkle Root constitutes the Execution Plan Commitment that users can incorporate in their transaction post-conditions.




During the execution, all the contract calls (static, dynamic and optimistic) are being collected, and attached to the transaction receipt:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...are collected and attached to the transaction receipt:


Proxy contracts act as a stable front-facing interface for users and other developers, providing a consistent address to interact with. Behind this interface, the actual business logic of the smart contract (the implementation contract) can be updated or upgraded as needed. This setup is crucial for fixing bugs or improving the contract's functionality over time without disrupting the service for users or the integrations built by other developers.

In the context of working with critical partners, especially those operating bridges between different blockchains or managing large amounts of liquidity, contract upgradability becomes a vital feature. This is because errors in these high-stakes environments can propagate across multiple blockchains, leading to extensive financial and operational damage. Thus, having the flexibility to upgrade contracts to address potential issues quickly is a key requirement for maintaining robust and secure blockchain ecosystems.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slight clarity improvement:

...issue quickly is key to maintaining robust and secure blockchain ecosystems.

}
```

This HTTP endpoint must also be able to execute any kind of function `readonly` , but also `readwrite` simulating writes to an in-memory store.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for added clarity what about:
...of function — readonly as well as readwrite — simulating writes to an in-memory store.

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.

10 participants