[adapter] run linker during publish flow #189
Conversation
| } | ||
| // TODO(https://github.com/MystenLabs/fastnft/issues/69): Run Move linker |
There was a problem hiding this comment.
I tried adding the run of the linker in this PR, but it needs an impl StateView that is inconvenient to create without some refactoring.
|
|
||
| // 1. Create a module that depends on a genesis module that exists, but via an invalid handle | ||
| let mut dependent_module = file_format::empty_module(); | ||
| // make `dependent_module` depend on `m` |
There was a problem hiding this comment.
| // make `dependent_module` depend on `m` | |
| // make `dependent_module` depend on `id_module` |
| .address_identifiers | ||
| .push(*id_module.self_id().address()); | ||
| dependent_module.module_handles.push(ModuleHandle { | ||
| address: AddressIdentifierIndex((dependent_module.address_identifiers.len() - 1) as u16), |
There was a problem hiding this comment.
Nit: I wonder if an inner function last_index returning u16 wouldn't help make this more readable.
There was a problem hiding this comment.
It definitely would... This code lives in the Move repo, otherwise I would go ahead and add it in the current PR.
|
|
||
| #[cfg(test)] | ||
| fn init_state() -> AuthorityState { | ||
| let (committee, authority_address, authority_key, store) = init_state_parameters(); | ||
| AuthorityState::new_without_genesis_for_testing( |
There was a problem hiding this comment.
I think we could just change init_state() to call new_with_genesis_modules instead of new_without_genesis_for_testing so that the initial state will have genesis. It doesn't hurt anyway.
There was a problem hiding this comment.
I think this makes sense, but doing this forces me to make init_state async and then async-ify the rest of the tests, which is a decently sized change. Would rather hold that for a different PR.
| // verifier may assume well-formedness conditions enforced by the Move verifier hold | ||
| let vm = MoveVM::new(natives) | ||
| .expect("VM creation only fails if natives are invalid, and we created the natives"); | ||
| let mut gas_status = get_gas_status(None) |
There was a problem hiding this comment.
Does this mean that linking through vm consumes gas? Is that something we want to track and deduct? (feel free to add TODO if that's the case)
There was a problem hiding this comment.
The VM doesn't do any gas metering on the publish code path--I could change the budget to (e.g.) Some(15) and the semantics would be the same. I'll add a comment to explain this.
Resolving a longstanding TODO to invoke the Move linker during the adapter's publishing flow. This will ensure that all packages are well-typed wr.t. their dependencies on-chain. In addition, this reorganizes the adapter publish code a bit to separate out the different kinds of checks from the update logic.
Resolving a longstanding TODO to invoke the Move linker during the adapter's publishing flow.
This will ensure that all packages are well-typed wr.t. their dependencies on-chain.
In addition, this reorganizes the adapter publish code a bit to separate out the different kinds of checks from the update logic. This should make it easier to invoke these checks from outside the publish path (e.g., from genesis or from the client) if needed.