[verifier] allow &TxContext in entry functions

[sblackshear]

Previously, we required entry functions that need a TxContext to take a &mut TxContext input. But an entry function that only reads the epoch or transaction sender does not need &mut. This change makes it possible to see whether an entry function may create new objects just by looking at the type signature. The new policy is thus:

• entry functions need not include a TxContext input
• If an entry function wants a TxContext input, it can declare either &TxContext or &mut TxContext according to its needs.

Tweaked the verifier to alow this, and updated some framework and example code accordingly.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated
explorer	🔄 Building (Inspect)			Jan 3, 2023 at 5:24PM (UTC)

2 Ignored Deployments

Name	Status	Preview	Comments	Updated
explorer-storybook	⬜️ Ignored (Inspect)			Jan 3, 2023 at 5:24PM (UTC)
wallet-adapter	⬜️ Ignored (Inspect)			Jan 3, 2023 at 5:24PM (UTC)

[sblackshear]

@tnowacki, interestingly, this change causes a compiler crash via sui move test:

-    public entry fun accept_transfer(r: &Registry, coin: &mut RCoin<Abc>, transfer: Transfer) {
+    public entry fun accept_transfer(r: &Registry, coin: &mut RCoin<Abc>, transfer: Transfer, _: &mut TxContext) { 

I think this is because there was test code that still passed the now-redundant context:

abc::accept_transfer(reg_ref, &mut coin, transfer, ctx(test));

Stack trace was

thread 'main' panicked at 'assertion failed: all_refs.is_empty()', /Users/shb/.cargo/git/checkouts/move-0639dd674f581c30/265e879/language/move-compiler/src/cfgir/borrows/state.rs:863:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
2022-12-29T00:04:24.158609Z ERROR telemetry_subscribers: panicked at 'assertion failed: all_refs.is_empty()', /Users/shb/.cargo/git/checkouts/move-0639dd674f581c30/265e879/language/move-compiler/src/cfgir/borrows/state.rs:863:9 panic.file="/Users/shb/.cargo/git/checkouts/move-0639dd674f581c30/265e879/language/move-compiler/src/cfgir/borrows/state.rs" panic.line=863 panic.column=9

Will try to create a smaller repro and submit an issue in the Move repo, but flagging here just in case.

[kchalkias]

From Issue description and this comment, it's not clear if we assume the input to an entry function can be any of &mut TxContext or &TxContext or we can even omit TxContext completely. (probably the word "optional" here causes that confusion)

[sblackshear]

Updated the PR description and will fix the comment as well.

[amnn]

LGTM! Love the legibility improvement for entry functions.

[amnn]

Consider re-using the earlier parameters?

Suggested change

	Err(format!(
	"Expected last (and at most second) parameter for {}::{} to be &mut {}::{}::{}, but found {}",
	"Expected last (and at most second) parameter for {}::{} to be &mut {}::{}::{} or &{}::{}::{}, but found {}",
	module.self_id(),
	INIT_FN_NAME,
	SUI_FRAMEWORK_ADDRESS,
	TX_CONTEXT_MODULE_NAME,
	TX_CONTEXT_STRUCT_NAME,
	SUI_FRAMEWORK_ADDRESS,
	TX_CONTEXT_MODULE_NAME,
	TX_CONTEXT_STRUCT_NAME,
	format_signature_token(view, &parameters[0]),
	))
	Err(format!(
	"Expected last (and at most second) parameter for {0}::{1} to be &mut {2}::{3}::{4} or &{2}::{3}::{4}, but found {5}",
	module.self_id(),
	INIT_FN_NAME,
	SUI_FRAMEWORK_ADDRESS,
	TX_CONTEXT_MODULE_NAME,
	TX_CONTEXT_STRUCT_NAME,
	format_signature_token(view, &parameters[0]),
	))

[tnowacki]

I think we need to update the comments in crates/sui-verifier/src/one_time_witness_verifier.rs too. which refers to &mut TxContext in some of it's error messages.

Similarly, should we allow &TxContext in init?
At one point, I removed the necessity of the transaction context at all in init but then decided that was silly since then your init can't do anything meaningful. Thoughts?