[#359] Fixed serialization of instructions with annotations
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:
-
encodeInstr
: extract var ann -
encodeVarNotedInstr
: extract type ann -
encodeNotedInstr
: encode both anns
It goes through:
-
encodeInstr
: extract type ann -
encodeVarNotedInstr
: falls through to the wildcard branch, callsencodeInstr 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:
-
encodeInstr
peels offInstrWithNotes
and callsencodeNotedInstr
-
encodeNotedInstr
falls through and goes back toencodeInstr
-
encodeInstr
peels offInstrWithVarNotes
and callsencodeVarNotedInstr
-
encodeVarNotedInstr
peels offInstrWithVarAnns
and callsencodeInstr
-
encodeInstr
peels offInstrWithVarNotes
and callsencodeVarNotedInstr
-
encodeVarNotedInstr
doesencodeWithAnns [] [] 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 (inencodeNotedInstr
).
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
Stylistic guide (mandatory)
-
My commits comply with the following policy. -
My code complies with the style guide.