Fix internal error in get_module #1380
Conversation
| ScErrorCode::InternalError, | ||
| "module cache missing contract", | ||
| &[], | ||
| Err(host.decorate_contract_code_storage_error( |
There was a problem hiding this comment.
What happens if a Wasm key is in the footprint, but the entry itself does not exist? IIUC we would still end up here and return the footprint error, which is not correct. The most comprehensive way to report an error here would probably be to do a storage lookup. If the Wasm is in the footprint, but storage returns None, then we can state that non-existent Wasm has been accessed. Otherwise, we can state that an entry outside of the footprint is being accessed. It's a bit of additional metered work, but since it only happens on the error path, I don't think it's an issue.
There was a problem hiding this comment.
Actually there is yet another scenario, where Wasm was uploaded after cache has been created. @graydon will look into that as well. Probably we should wait with this PR until then, because I'm not sure what the error logic should be.
|
Added PR #1382 which covers this more thoroughly. Closing in favor of it -- reopen if you disagree! |
This covers a two cases I failed to cover in #1375 that @dmkozh and @sisuresh have identified: - User calls contract without putting wasm for call in footprint or storage - User calls `upload_contract` (or its equivalent, `update_current_contract_wasm`) mid-execution and then calls the updated-or-new contract As well as a third case that is, as far as I know, not possible; but it seems to me it doesn't hurt to be defensive since someone could make it possible in the future fairly easily and forget to cover this case in the cache code: - User deletes wasm in transaction, and then tries to call it. As always, there is probably room for a bit more testing here, but I wanted to get these cases covered and fixed asap. This should make-redundant PR #1380
What
get_moduleassumes the wasm blob was added to the module cache when it iterated over storage, but the wasm blob could be missing from storage if it wasn't included in the footprint. Return the same error we return today when the wasm blob is missing.