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

[Clarity] rules for determining which Clarity version to run #3131

Closed
jcnelson opened this issue May 9, 2022 · 5 comments
Closed

[Clarity] rules for determining which Clarity version to run #3131

jcnelson opened this issue May 9, 2022 · 5 comments

Comments

@jcnelson
Copy link
Member

jcnelson commented May 9, 2022

Now that there are multiple versions of Clarity, with different keywords and different interpretations of existing keywords, we need to determine the rules for how (at-block), contract-call? and smart contract deployment should work. I suggest the following:

  • The Clarity version used in a transaction is immutable over the transaction's lifetime. Moreover, the Clarity version used is determined early -- i.e. at or before the instantiation of the transaction's GlobalContext.

  • A ContractCall transaction runs with the Clarity version of the contract it calls, not the current epoch. Similarly, a SmartContract transaction runs with the Clarity version it indicates in its pragma ([Chainstate] SmartContract tx wire format to indicate which Clarity version to use #2719), not the current epoch (the current epoch is only used to infer the Clarity version if the SmartContract payload indicates to do so). If the contract is deployed with Clarity1 rules, then all top-level contract calls into it use Clarity1. If a contract is deployed with Clarity1 rules, and it calls other contracts as part of its code-body evaluation, then these contract-calls follow Clarity1 rules as well. Same goes for the use of Clarity2.

  • The closure in (at-block) runs with the Clarity version of the caller, not the target block. So, if a Clarity2 contract calls (at-block), its code body runs with Clarity2 rules even if the block was mined before Clarity2 was allowed. Similarly, if a Clarity1 contract calls (at-block), its code body runs with Clarity1 rules even if the block was mined when Clarity2 was allowed. If the closure fails for any reason -- even via CheckErrors resulting from the Clarity version's rules rendering the closure's code invalid -- the error is always (re-)interpreted as a runtime error ([clarity] All check errors must be handled as runtime errors #3107) so the transaction can still be mined.

  • The closure in (at-block) can only call other contracts that have been successfully instantiated. This precludes calling a contract that uses Clarity2 keywords but was deployed in epoch 2.05 or earlier via (at-block) -- the call should fail even if the contract could have worked in Clarity2, because the contract itself should not have passed the analysis check when its transaction was mined.

@jcnelson
Copy link
Member Author

jcnelson commented May 9, 2022

@kantai does this make sense to you?

@kantai
Copy link
Member

kantai commented May 9, 2022

The Clarity version used in a transaction is immutable over the transaction's lifetime. Moreover, the Clarity version used is determined early -- i.e. at or before the instantiation of the transaction's GlobalContext.

I don't think this is feasible: a Clarity1 contract, invoked by a Clarity2 contract, must execute as Clarity1. This is how it is currently implemented in next: the ContractContext determines the ClarityVersion, and is the only place where the VM can obtain the version. Otherwise, contracts would be running in variable versions, which would be pretty confusing, I think.

I think this answer also makes it pretty clear how your second point should be interpreted: ContractCall invokes a contract, which specifies its ClarityVersion. Contract publish events don't "run at a Clarity version", rather they instantiate a Contract as a Clarity2 contract or a Clarity1 contract, as specified by the transaction (or defaults).

The closure in (at-block) runs with the Clarity version of the caller, not the target block. So, if a Clarity2 contract calls (at-block), its code body runs with Clarity2 rules even if the block was mined before Clarity2 was allowed.

Yes, I think that's right, but I'd still frame this specification in terms of the contract that the at-block appears in: in a Clarity2 contract, the at-block evaluates as Clarity2.

The closure in (at-block) can only call other contracts that have been successfully instantiated. This precludes calling a contract that uses Clarity2 keywords but was deployed in epoch 2.05 or earlier via (at-block) -- the call should fail even if the contract could have worked in Clarity2, because the contract itself should not have passed the analysis check when its transaction was mined.

I'm not sure that this is true. If an at-block contains a trait invocation (which I think may be possible in Clarity1 due to a bug in the at-block checker, even though any non-read-only actions will actually be rolled back), it could invoke a trait-bound contract. But either way, I still think there's a relatively clear way to apply the versioning rules -- any called contract runs in the version that it specifies.

So what I'd propose as the versioning rules:

  1. Contracts run at the ClarityVersion that the contract specifies. This is defined at the time of contract publish (either through specification in the transaction or by defaults). contract-calls will lead to a change in ClarityVersion if a Clarity2 contract calls a Clarity1 contract.
  2. at-block closures execute at the version of their contract (like any other code block).
  3. Epoch versions, however, are defined at the block level (like in 2.05 vs. 2.0)

@jcnelson
Copy link
Member Author

That sounds much better than what I was suggesting. Thank you for that.

What do you think about runtime costs? Which .costs contract gets used? Do we just use whatever one is active as of the block in which the transaction is run, or will Clarity1 code need a different .costs contract for Clarity2? Is this still the case across an at-block boundary -- i.e. the costs that was active in the top-level transaction is what gets used to assess runtime resource usage within the closure (instead of the costs of the target block)?

Also, how do you see this affecting PoX? If I have an at-block closure that invokes the old .pox contract after .pox-2 is live, then even though .pox-2 might not exist in the target block, does the at-block closure still fail?

@kantai
Copy link
Member

kantai commented May 10, 2022

What do you think about runtime costs? Which .costs contract gets used? Do we just use whatever one is active as of the block in which the transaction is run, or will Clarity1 code need a different .costs contract for Clarity2? Is this still the case across an at-block boundary -- i.e. the costs that was active in the top-level transaction is what gets used to assess runtime resource usage within the closure (instead of the costs of the target block)?

Yep -- the costs are defined by the block: the cost tracker is instantiated only once per block, and it uses the epoch defined costs contract (as the default costs: if a cost vote passes, it uses the results of that for a given cost function). This is true through at-block, contract-call, and irrespective of the Clarity version.

Also, how do you see this affecting PoX? If I have an at-block closure that invokes the old .pox contract after .pox-2 is live, then even though .pox-2 might not exist in the target block, does the at-block closure still fail?

It won't fail as currently implemented in next: the .pox methods check the v1_unlock_height (set as a constant in the database), and the current burnchain block height (this changes during an at-block) to trigger a failure. So, read-only methods of the .pox contract will still be invocable through at-block.

@kantai
Copy link
Member

kantai commented May 10, 2022

Re: the pox and at-block behavior, I don't have strong feelings as to whether or not the current behavior is the desired behavior, or if something like a runtime error for all PoX contract invocations in at-block would be better. I'm inclined to think it doesn't matter much either way: there's no danger in allowing those historic read-only lookups, and it may be the case that some contract is particularly interested in fetching historic lock data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

2 participants