use-ink / contracts-ui

Web application for deploying wasm smart contracts on Substrate chains that include the FRAME contracts pallet
https://contracts-ui.substrate.io/
GNU General Public License v3.0
61 stars 45 forks source link

fix: pretty print dry-run results #441

Closed statictype closed 1 year ago

statictype commented 1 year ago

related to https://github.com/paritytech/contracts-ui/issues/421

netlify[bot] commented 1 year ago

Deploy Preview for contracts-ui ready!

Name Link
Latest commit 5930c4cd9774e55eef8dcac07d509b4c12ffe88f
Latest deploy log https://app.netlify.com/sites/contracts-ui/deploys/641aeead15d2960008693105
Deploy Preview https://deploy-preview-441--contracts-ui.netlify.app/
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

peetzweg commented 1 year ago

I tried to review this yesterday a bit. Code wise it looks good to me. However, I wasn't able yet to find a contract which has a struct returned like cmichi described. Will reach out to grab the contract or could just probably adapt the flipper one. 🤔

statictype commented 1 year ago

try this one. it returns a struct too.

peetzweg commented 1 year ago

Not to sure if this is what @cmichi asked for in the ticket. Proper strings now lose the quotation marks ". Sure Numbers are now without quotes but the dry-run results is not not even valid JSON.

Frame 1(1)

When reading the ticket I was more imagining something like this. Where the toString() representation contains the actual rust type. Not sure how this is commonly done but it could look like this.

{
    "name": "String('This is a String')",
    "subject": "Hash(0x0000000000000000000000000000000000000000000000000000000000000000)",
    "bids": "Vec[Vec[Option[AccountId('5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY'),uint128('100')]]",
    "terms": [
        "Number('0')",
        "Number('1')",
        "Number('2')"
    ],
    "status": "Status('OpeningPeriod')",
    "finalized": "Boolean(true)",
    "vector": "Vec<u8>('A Vec<u8> :shrug:')"
}
statictype commented 1 year ago

No that's not what we want. We need to show the dry-run outcome as it comes back from chain. We don't have the type information of the individual struct fields there. It could be possible but at but it is an insane computation given that the result can be anything. What i wanted to achieve with this PR is to remove all the quotes that come with JSON.stringify. This means also removing quotes around strings, as a tradeoff We don't need this to be valid JSON, it is just printed to the screen, not used anywhere.

statictype commented 1 year ago

We can explore better ways of displaying struct results, but for now this should do. Like for example https://github.com/stoplightio/json-schema-viewer or https://www.npmjs.com/package/react-json-view These quotes were introduced in the WeightV2 PR, where I changed the output decoding helper. Basically now we return to the way it was before (even if not ideal)

statictype commented 1 year ago

toHuman() decodes anything, it is available on any Codec interface. it has no type information whatsoever though. in the case of structs it returns an object which needs to be stringified in order to be displayed to the screen. the actual result we want to display often comes nested so just calling toHuman on the result isn't enough. I added a different approach which strips strings only from the object keys