Skip to content

[#359] Fixed serialization of instructions with annotations

Diogo Castro requested to merge diogo/#359-more-bug-fixes into master

Description

Problem: When we serialize EMPTY_SET :ta @va int, the type ann :ta does not get serialized.

{
  "prim": "EMPTY_SET",
  "args": [
    {
      "prim": "int"
    }
  ],
  "annots": [
    "@va"
  ]
}

Looking at the structure of encodeInstr + encodeVarNotedInstr + encodeNotedInstr, it looks like it expects an instruction to be wrapped with meta-instructions in this order:

InstrWithVarNotes $
  InstrWithVarAnns $
    InstrWithNotes $
      EMPTY_SET

However, in Michelson.TypeCheck.Helpers -> wrapWithNotes -> addNotesOneVarAnn, we wrap the instruction in this order:

InstrWithNotes $
  InstrWithVarNotes $
    InstrWithVarAnns $
      EMPTY_SET

So, in practice, instead of going through:

  1. encodeInstr: extract var ann
  2. encodeVarNotedInstr: extract type ann
  3. encodeNotedInstr: encode both anns

It goes through:

  1. encodeInstr: extract type ann
  2. encodeVarNotedInstr: falls through to the wildcard branch, calls encodeInstr instr, and loses the type ann.

One solution would be to fix the order in which we wrap the instruction in meta-instructions in Michelson.TypeCheck.Helpers.

But a better solution is to fix the encode* loops so that the order of the meta-instructions does not matter.

Besides, these encode* loops are in dire need of refactoring, they are way too complex and prone to bugs.

For example, when serializing a PAIRN instruction, this is what happens:

  1. encodeInstr peels off InstrWithNotes and calls encodeNotedInstr
  2. encodeNotedInstr falls through and goes back to encodeInstr
  3. encodeInstr peels off InstrWithVarNotes and calls encodeVarNotedInstr
  4. encodeVarNotedInstr peels off InstrWithVarAnns and calls encodeInstr
  5. encodeInstr peels off InstrWithVarNotes and calls encodeVarNotedInstr
  6. encodeVarNotedInstr does encodeWithAnns [] [] vns $ encodeInstr i

The code now works as follows:

  • encodeInstr will serialize any instructions that are not wrapped in meta-instrs
  • if a instruction is wrapped in any meta-instr, it'll go through encodeInstr -> encodeVarNotedInstr -> encodeNotedInstr. As it goes down, we peel off all meta-instr layers, and accumulate all type/var/field anns info, and handle everything at the very bottom (in encodeNotedInstr).

No more jumping back and forth between encode* instructions.

In the process of fixing+refactoring this, I also ended up uncovering and fixing a few more bugs (also related ot the serialization of instructions with annotations).

More details in the commit messages.

Related issue(s)

Resolves part of #359 (closed)

Checklist for your Merge Request

Related changes (conditional)

  • Tests (see short guidelines)

    • If I added new functionality, I added tests covering it.
    • If I fixed a bug, I added a regression test to prevent the bug from silently reappearing again.
  • Documentation

    • I checked whether I should update the docs and did so if necessary:
    • I updated changelog files of all affected packages released to Hackage if my changes are externally visible.

Stylistic guide (mandatory)

Edited by Diogo Castro

Merge request reports