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

MaslLibrary should validate the code it compiles #1295

Open
hackaugusto opened this issue Mar 26, 2024 · 3 comments
Open

MaslLibrary should validate the code it compiles #1295

hackaugusto opened this issue Mar 26, 2024 · 3 comments

Comments

@hackaugusto
Copy link
Contributor

#[test]
fn call_invalid_loc_store() {
    ModuleAst::parse(
        "\
    proc.invalid.1
        loc_store.1
    end
    ",
    )
    .unwrap();
}
  • The invalidated library above is serialized to disk
  • The library is read with the invalid opcode and added to the ModuleProvider with the invalid opcode
  • Later on:
    • for re-exports referencing a module with broken code
      • code that re-exports any procedure from the invalid module will fail to compile
      • the compilation will fail on the first re-export, which does NOT have to be related to the invalid procedure
      • the compilation error is swallowed
      • the result of re-exporting a invalid procedure is AccountCodeAssemblerError(ReExportedProcModuleNotFound(..., name: ProcedureName { name: "receive_asset" })), which is very deceiving.
    • for execs referencing a module with broken code
      • the compilation will fail with AccountCodeAssemblerError(ParamOutOfBounds(1, 0, 0)), which is the right error but no useful context
@hackaugusto
Copy link
Contributor Author

I split the above into multiple, smaller issues. Some of them may already be worked on, or even fixed on some of the existing PRs.

@bitwalker
Copy link
Contributor

I wonder if this issue is going to remain relevant once we switch to distributing MAST instead (which I expect to happen shortly after 1277 is merged, as the next in that series of changes):

  • We won't need to parse/compile the code, since that's already been done
  • We will need to validate the MAST, but we have to do most of that to properly deserialize it anyway. I think the main thing we'll probably have to do in addition to the validation we already be doing during deserialization is validating the MAST hashes are correct.

As part of that switch, the new package format would also be introduced and used in place of MASL libraries. As a result, I'm not sure MASL libraries will even need to exist at that point. So I suppose my question is: do we try to address these issues before that happens, or do we address them in the switch to MAST?

@bobbinth
Copy link
Contributor

My preference would be to fix this as a part of transition to MAST-based libraries.

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

No branches or pull requests

3 participants