pwasm-abi icon indicating copy to clipboard operation
pwasm-abi copied to clipboard

WIP: move from serde_json to serde_json_core

Open Pzixel opened this issue 6 years ago • 10 comments

I have created an issue that serde_json clashes in no_std scenarios, but library authors said that they don't support no_std.

std feature should be removed from the crate (to not confuse people that it supports some kind of no_std)

See more details here https://github.com/serde-rs/json/issues/463

So I tried to add required functionallity into the package that should be a no_std replacement for serde_json - serde_json_core.

It doesn't work yet (so I added WIP) but it probably would be helpful to have this PR here. I appreciate any help because now I don't quite understand what's wrong with code. All tests passed for alloc types, but when you actually use them you encounter several errors in macro invocations.

Pzixel avatar Jul 19 '18 13:07 Pzixel

Looks like serde bug: https://github.com/serde-rs/serde/issues/1339

Pzixel avatar Jul 19 '18 13:07 Pzixel

There is no need to support json in no-std, because json is generated on compile time only and json serialization/deserialization never makes it to the wasm binary

NikVolf avatar Jul 23 '18 17:07 NikVolf

@NikVolf having dependency on serde_json makes impossible to use crates like bincode, see #54 . It's because you cannot reference serde in std and no_std scenarios. This is limitation of existing cargo, and it's unlikely that it will be changed in the future.

In my case I want to bincode some types in my contract (basically to implement generic object saving in pwasm-abstractions library), but I can't do that currently.

Pzixel avatar Jul 23 '18 18:07 Pzixel

I have reread the derive subcrate source code. It seems that json generation should be moved to build.rs to make serde build-time dependency. It looks like eth_abi macro voliates SRP principle: it generates both source code and JSON. I see that it uses the same AST to do two things, but I actually think that it should be separated into two methods, and one of them moved into build.rs.

Pzixel avatar Jul 26 '18 21:07 Pzixel

I have reread the derive subcrate source code. It seems that json generation should be moved to build.rs to make serde build-time dependency. It looks like eth_abi macro voliates SRP principle: it generates both source code and JSON. I see that it uses the same AST to do two things, but I actually think that it should be separated into two methods, and one of them moved into build.rs.

The argument behind it is following: ".json ABI is an artefact of compilation". That means we should maintain the following requirement: in case of compilation failure neither .json ABI nor Wasm binary should change.

lexfrl avatar Jul 27 '18 07:07 lexfrl

But it currently makes serde_json runtime dependency. As i have shown it doesn't allow user to use serde in the contract. If you have other ideas i'd like to try them too.

Just try to create a contract, that serialize the tuple of U256 into byte array and save it to bc. You will face the error.

Pzixel avatar Jul 27 '18 08:07 Pzixel

I think the ideal solution would be to make a generic build tool, as an extension of cargo tool (will take the same arguments as cargo and some additional), which will yield the .json ABI as well. We could call it pargo, for example.

lexfrl avatar Jul 27 '18 08:07 lexfrl

Sounds promising

Pzixel avatar Jul 27 '18 08:07 Pzixel

I think the ideal solution would be to make a generic build tool, as an extension of cargo tool (will take the same arguments as cargo and some additional), which will yield the .json ABI as well. We could call it pargo, for example.

@fckt Is this idea still a thing? If so where should we collect information about it? Maybe a new issue?

Could a cargo plugin be helpful here?

Robbepop avatar Oct 15 '18 18:10 Robbepop

@Robbepop yeah, still actual. I'm thinking to make it as a subcommand like cargo pwasm-build I've started implementing it, but had to focus on Parity Signer recently, due to a release matters. This refactoring was the first move into that direction.

lexfrl avatar Oct 16 '18 07:10 lexfrl