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

SPIKE: Comprehensive testing of Michelson Encoder #2931

Open
ac10n opened this issue Apr 23, 2024 · 0 comments
Open

SPIKE: Comprehensive testing of Michelson Encoder #2931

ac10n opened this issue Apr 23, 2024 · 0 comments
Milestone

Comments

@ac10n
Copy link
Contributor

ac10n commented Apr 23, 2024

In ticket #2929, users observed an unexpected computation of the idx for nested Pair and Or Tokens.
The specific situation creating this was: A pair inside or that's inside another pair.
User expected that items inside the innermost pair restart from 0, but they started from 3.

As there is no formal specification for mapping Michelson data to javascript objects, this is not a bug, but it's indeed an unexpected behaviour. There could be many more of these, especially:

  1. When combining complex data like Pair, Or, List, etc.
  2. When some items inside Pair or Or have annots and some others don't

In particular, for 2, deciding the right behaviour can be tricky, for instance, consider how a pair should be formatted into an object when the first two have annots, and the last two don't:

{
  annot1: 'Value0',
  annot2: 'Value1',
- 2: 'Value2'
+ 0: 'Value2',
- 3: 'Value3',
+ 1: 'Value3', 
}

In the above case, we should decide if the counting should include the ones with annots, or be exclusively limited to the ones without annots.
In both cases, if an item has annots that's a number which will collide with our counting will currently be shadowed. We need to make an explicit decision:

  • Jump over those
  • Start counting from the largest one

I suggest we need:

  1. A specifications document that precisely defines the mapping in all possible cases
  2. A test that algorithmically generates schema, data, and the expected resulting javascript object according the specs and verifies the MichelsonEncoder package's implementation
  3. In case there are serious breaking changes, we might need a flag that controls if the code behaves in a backward-compatible way, or according to the new "correct" way

The specs document will be trivial for everything except Pair, Or, but for these two, it needs a lot of attention to make sure all possibilities are covered.

@hui-an-yang hui-an-yang added this to the v20+ milestone May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: To do
Development

No branches or pull requests

2 participants