Allow to pass arguments as a dictionarry

[volovyks]

In Rust and AS SDKs we are passing function arguments as dictionaries. This PR is intended to move from arguments passed as an array, to arguments passed as dict.

Before:

await ali.call(jsvm, 'call_js_contract', encodeCall(testContract.accountId, 'set_values', ['newVal1', 'newVal2', 'newVal3']), { attachedDeposit: '100000000000000000000000' });

After:

await ali.call(jsvm, 'call_js_contract', encodeCall(testContract.accountId, 'set_values', { param1: 'newVal1', param2: 'newVal2', param3: 'newVal3' }), { attachedDeposit: '100000000000000000000000' });

Note: JS has two approaches to pass arguments.
Positional:

function rand(param1, param2) {...}
const val1 = '';
const val2 = '';

rand(val1, val2)

And named:

function rand({ param1, param2 }) {...}
const val1 = '';
const val2 = '';

rand({param1: val1, param2: val2}) {}

With the current design, we will force users to use named arguments in the contract interface. Not sure if it is possible to add support for both named and positional parameters.

Usually named parameters are considered a good practice, since they are allowing flexible and extendable API. Also, the code becomes easier to read.

Thoughts? cc @ailisp @BenKurrek @gagdiez @MaximusHaximus @ChaoticTempest

P.S. Still need to fix a few tests. JS refactoring is a terrible process :/

[BenKurrek]

This is great - definitely a good change. I'll take a look at the PR tomorrow :)

[ChaoticTempest]

Is it possible to modify each method, and replace them with one that already inlines { } between all the arguments? We can get rid of forcing the users to constantly write these brackets otherwise and would make the classes look a lot simpler

[ailisp]

It is possible as we have full access to AST. However I think it's controversial to do so - it's a hidden magic turns the user contract to non standard JavaScript.

[ChaoticTempest]

hmm, but would there be a case where the contract is used as standard javascript though? We don't have any unit tests, and workspaces just makes RPC calls anyways. And the thing is, it's all very magicful anyways when we're sending the arguments across the wire onto the network. Plus if we ever decide to support borsh, the brackets {} would make it seem weird in that case

[volovyks]

During contract development you are getting errors. The first place you go is *contract".js file, because error givese you line where it happened. Long magical code will be unfortunate here.

[volovyks]

Not sure how it will affect borsh.

[ChaoticTempest]

The first place you go is *contract".js file, because error givese you line where it happened. Long magical code will be unfortunate here

aren't we generating a separate contract.js file anyways? So error output (or the line the error occurred) would just match up with the generated file then or am I missing something? The generated file should have these inplaced {} too so I don't see an issue there.

Not sure how it will affect borsh

Not entirely sure how the structure of borsh could like, but the brackets {} would match more closely to JSON, while borsh could just be any plain old object that's not necessarily a dictionary. Depends on how we want to serialize/deserialize such an object

[ailisp]

Plus if we ever decide to support borsh, the brackets {} would make it seem weird in that case

When you make this kind of signature:

function rand({ param1, param2 })

argument encoding with borsh, it can't work since the order of param1 and param2 is not guaranteed. You can use a borsh-encoded hashmap which has param names and values, but that violates to you want a dense, succinct benefit from borsh. So practically it has to be:

function rand(param1, param2)

Therefore we have named parameter for json, and non named parameters for borsh. With Serhii's design, both can be achieved by expressing in standard JS:

function rand({ param1, param2 })
function rand(param1, param2)

If we modify code generation to auto add { }, user cannot achieve the later case.

[ChaoticTempest]

Therefore we have named parameter for json, and non named parameters for borsh. With Serhii's design, both can be achieved by expressing in standard JS:

function rand({ param1, param2 })
function rand(param1, param2)

I'd argue that this would force the user to know the intricacies of one over the other.

If we modify code generation to auto add { }, user cannot achieve the later case.

How parametrizable are the JS decorators? Because if they allow specifying parameters, or allowing adding additional ones, I can see us just specifying the serialization format we want the arguments to be such as:

@call
@borsh
function rand(param1, param2) {}

that way we don't force the user to know all the different details of why they'd need one over the other for specifying a serialization format. Also, RS SDK does this too like so:

fn rand(&self, #[serializer(borsh)] param1, #[serializer(borsh)] param2) {}

But I'm not saying that this necessarily the best path to take, just noting that forcing the users into a specific syntax might have some unintended complexities later on if we do decide to hide some of these things. You guys have the final say for this.

Also, I'm noticing now that I wrote out the rust one, the JS SDK doesn't have per-parameter serialization format right now since JSON is the default. If we do decide to support such a thing in the future, I think the syntax of {param1, param2, ...} might be unwieldy to specify which parameters have what serialization format, but we can think about that later since we won't stabilize 1.0 quite yet

[ailisp]

Good points.

@call
@borsh
function rand(param1, param2) {}

seems quite good.

If we do decide to support such a thing in the future, I think the syntax of {param1, param2, ...} might be unwieldy to specify which parameters have what serialization format, but we can think about that later since we won't stabilize 1.0 quite yet

The problem is that, even with above @borsh you cannot specify per paramter serialization - js decorators can only on function/method/class, not parameters.

Also I want to know how useful is per-param serialization, e.g.:

fn rand(&self, #[serializer(json)] param1, #[serializer(borsh)] param2) {}

What is the use case of it, is that only happen at callback function of a borsh contract cross contract call a json contract?

[ailisp]

Assume we want per-param serialization, given JS decorators' limitation: can only on function/method/class, not parameters. we possibly do this:

@call
@customSerializer({'param2': 'borsh'})
function rand(param1, param2) {}

In this case either rand(param1, param2) or rand({param1, param2}) should work. So Serhii's approach is still good

[ailisp]

I think this is a great and smart approach! it is true that's more flexible and extendable. Also you reused JS's language feature, should be natural and no surprise to the developer. This approach also reduce the complexity of my proposed approach: walk the AST to record param names and generate code to map dict-arguments into positional argument. So I think your approach is better!

[gagdiez]

loving the idea. I think we MUST force users to use named parameters, in that way, if I forgot to pass a parameter, the contract will panic with a meaningful error. Passing values by parameter is highly error-prone, specially in JS, where values are not typed.

[ailisp]

Looks great, I think this is the best solution we can achieve before the consensus. Whether this or @ChaoticTempest 's suggest is better is controversial, let's follow up the discussion after this PR