AntelopeIO / leap

C++ implementation of the Antelope protocol
Other
116 stars 69 forks source link

`optional` parameters and `cleos` #1959

Open YaroShkvorets opened 12 months ago

YaroShkvorets commented 12 months ago

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 commented 11 months ago

@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 commented 11 months ago

@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.

spoonincode commented 10 months ago

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.