yeagerai / genlayer-simulator

MIT License
16 stars 8 forks source link

feat: migrate to new calldata #558

Closed kp2pml30 closed 1 month ago

kp2pml30 commented 1 month ago

Fixes \<no issue>

What

Why

Testing done

Decisions made

Checks

Reviewing tips

For now there is a problem with "json" input by user for obvious reasons. I can allocate some time to fix them, but I am not great in reactive ui's

User facing release notes

New calldata string representation is slightly different from json:

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 88.51775% with 55 lines in your changes missing coverage. Please review.

Project coverage is 19.60%. Comparing base (506ebd4) to head (699ad16). Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...src/components/Simulator/ConstructorParameters.vue 0.00% 17 Missing :warning:
frontend/src/calldata/encoder.ts 90.85% 15 Missing :warning:
frontend/src/hooks/useContractQueries.ts 7.14% 13 Missing :warning:
...nd/src/components/Simulator/ContractMethodItem.vue 0.00% 8 Missing :warning:
frontend/src/calldata/parser.ts 99.13% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #558 +/- ## ========================================== + Coverage 15.35% 19.60% +4.24% ========================================== Files 112 116 +4 Lines 7950 8381 +431 Branches 187 274 +87 ========================================== + Hits 1221 1643 +422 - Misses 6653 6662 +9 Partials 76 76 ```

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

cristiam86 commented 1 month ago

Changes in general look good. I'm super lost around the encoding part

I have checked this branch and I cannot use the simulator properly, when trying to run write methods I get

Uncaught (in promise) Error: Error writing to contract
    callWriteMethod useContractQueries.ts:220
    handleCallWriteMethod ContractMethodItem.vue:58
    callWithErrorHandling runtime-core.esm-bundler.js:199
    callWithAsyncErrorHandling runtime-core.esm-bundler.js:206
    emit runtime-core.esm-bundler.js:6306
    createSetupContext/get emit/< runtime-core.esm-bundler.js:8016
    0 Btn.vue:46
    callWithErrorHandling runtime-core.esm-bundler.js:199
    callWithAsyncErrorHandling runtime-core.esm-bundler.js:206
    invoker runtime-dom.esm-bundler.js:722

I think the UI it's also broken in some places image

Extra

I also want to remark that this PR has big breaking changes that will impact:

  • GenLayerJS
  • users upgrading

@AgustinRamiroDiaz Can you please disclose the contract and the method that you are calling to get this error? I've tried different ones and they all succeed.

kp2pml30 commented 1 month ago

Actually I found out that probably what we mean by address is 20 bytes and not 32 as in this PR. It is sort of confusing:

kp2pml30 commented 1 month ago

Sorry I messed up this PR.... It won't reopen

I am sorry this happened: new PR is #565