stacks-network / clarity-wasm

`clar2wasm` is a compiler for generating WebAssembly from Clarity.
GNU General Public License v3.0
14 stars 15 forks source link

Rewrite of tuple consensus-buff deserialization #434

Closed Acaccia closed 4 months ago

Acaccia commented 4 months ago

This is the first part of completing #307 .

This PR rewrites completely the tuple deserialization function to allow for the missing functionalities. For now, the missing functionalities implemented are

To satisfy those conditions, the new algorithm in pseudo-code became

let bitset = Bitset::new();
for key in serialized_bytes {
    let n = find_index(key)
    switch n {
        case 1:
            if bitset.contains(key) { return None; }
            handle_parsing_of_value_1;
            bitset.insert(key);
            break;
        case 2:
            if bitset.contains(key) { return None; }
            handle_parsing_of_value_2;
            bitset.insert(key);
            break;
        ...
        default:
            check_valid_skippable_key();
            check_valid_skippable_value();
            break;
    }
}
if bitset.full() { return Some(result) } else { return None };

These contains new functionalities that had to be created:

  1. The bitset is a list of i32 locals, the logic to handle it is directly inside the deserialization algo.
  2. The find_index function is a binary search in a list of string. It is implemented in the standard, with another function to compare clarity names (there are also unit tests for this function).
  3. The switch-case construct is a bit awkward in Wat, since it doesn't have _label_s and _goto_s instructions. So it's a succession of labelled blocks, with the inner one being the switch and using a br_table instruction to jump after the block labelled like the currently parsed field (I suggest compiling a few examples to see what it looks like).
  4. The default case where we switch extra fields is a TODO for now but is nearly complete already. I decided to cut it from this PR and add it in a future one because it will require quite a lot of code to handle the testing.
codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 95.01558% with 16 lines in your changes missing coverage. Please review.

Project coverage is 86.23%. Comparing base (93eace4) to head (f46c59e).

Files Patch % Lines
clar2wasm/src/deserialize.rs 95.66% 10 Missing and 3 partials :warning:
clar2wasm/src/wasm_generator.rs 85.71% 1 Missing and 2 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #434 +/- ## ========================================== + Coverage 86.20% 86.23% +0.02% ========================================== Files 44 44 Lines 19406 19557 +151 Branches 19406 19557 +151 ========================================== + Hits 16729 16865 +136 - Misses 1212 1223 +11 - Partials 1465 1469 +4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Acaccia commented 4 months ago

@krl here is the crosscheck testing for serialization/deserialization: https://github.com/stacks-network/clarity-wasm/blob/93eace432d3cac709c3ade3998ee703b6a7aa020/clar2wasm/tests/wasm-generation/values.rs#L17-L27

There will be more testing for the new features in the next PR that will actually implement the new features.