Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optional parameters and cleos #1959

Open
YaroShkvorets opened this issue Dec 7, 2023 · 3 comments
Open

optional parameters and cleos #1959

YaroShkvorets opened this issue Dec 7, 2023 · 3 comments
Labels
bug Something isn't working discussion 👍 lgtm

Comments

@YaroShkvorets
Copy link
Contributor

Noticed something weird.
If I have an action with optional parameter(s):

#include <eosio/eosio.hpp>

using namespace eosio;

class [[eosio::contract("test")]] test : public contract {
public:
    test( name receiver, name code, datastream<const char*> ds ):contract(receiver, code, ds) {};

    [[eosio::action]]
    void testaction( std::optional<name> user ) {
        require_auth( get_self() );
    }
};

Then I can pass into cleos whatever I want as action parameters and it will accept it:

❯ cleos push action myaccount testaction '{"asdfasdfa": {"asdf": "asdf"}}' -p myaccount
executed transaction: 32a4c4ddc991f3fec2cc94e9c042079ed0653a1512131403a354644aa36e81ba  96 bytes  295 us
#        myaccount <= myaccount::testaction           {"user":null}

I thought it's supposed to validate against ABI.

Caught me by surprise after I changed action signature in my contract but my test script continued to work fine without producing an error.

❯ cleos version full
v5.0.0-rc2-f77423b749e05ee2c35e16ea69c07d67f5dab950
@bhazzard
Copy link

@heifner I think this one needs a quick look to determine if this is expected behavior or a bug, and what the implications are if it is a bug.

@heifner
Copy link
Member

heifner commented Dec 12, 2023

@bhazzard this is a bug. I was able to reproduce with the provided contract. std::optional<name> allows additional fields (which is what asdfasdfa is assumed to be. If you use just name then it verifies the ABI as expected. Not sure how difficult this will be to fix.

@bhazzard bhazzard added bug Something isn't working 👍 lgtm and removed triage labels Dec 12, 2023
@spoonincode
Copy link
Member

I'm not so sure I'd consider this a bug. If it is to be considered a bug, it's nothing specific to an optional type in ABI but rather how JSON works across the entirety of Leap: unknown keys are silently ignored. (fwiw, from memory, this is an important difference in how abieos handles JSON)

For some other examples, consider

cleos push action eosio.token transfer '{"foo":"bar", "to":"bro","quantity":"1.0000 EOS","monkey":"banana","from":"spoon","memo":""}' -p spoon@active
executed transaction: 205f775f1b488e2222199852fb1e28e0e32151d341aa9a9d821ee8e902012aa8  128 bytes  562 us
#   eosio.token <= eosio.token::transfer        {"from":"spoon","to":"bro","quantity":"1.0000 EOS","memo":""}
#         spoon <= eosio.token::transfer        {"from":"spoon","to":"bro","quantity":"1.0000 EOS","memo":""}
#           bro <= eosio.token::transfer        {"from":"spoon","to":"bro","quantity":"1.0000 EOS","memo":""}
warning: transaction executed locally, but may not be confirmed by the network yet         ]

foo and monkey were ignored.

Also this applies to API endpoints as well,

curl --data '{"foo":"bar", "account_name":"spoon", "monkey":"banana"}' http://127.1:8888/v1/chain/get_account
{"account_name":"spoon","head_block_num":2004,"head_block_time":"2024-02-06T16:16:20.500","privileged":false,"last_code_update":"1970-01-01T00:00:00.000","created":"2024-02-06T16:01:15.000" (snip)

Again, foo and monkey were ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working discussion 👍 lgtm
Projects
Status: Todo
Development

No branches or pull requests

5 participants