Remove default value from the optional `new_fee_rate` member in `samet_fund_update_operation`

[abitmore]

If the type of a member variable is Optional, setting a non-standard default value will cause serialization / deserialization problems.

Follow-up of https://github.com/bitshares/bitshares-core/pull/2469.

To reproduce:

unlocked >>> sign_transaction { "operations": [[66,{ "fee": { "amount": 100000, "asset_id": "1.3.0" }, "owner_account": "1.2.3833", "fund_id": "1.20.1", "delta_amount": {"asset_id":"1.3.0","amount":"2000000"}, "new_fee_rate": null}]] } true Execution error: missing required active authority: Missing Active Authority 1.2.3833 ... "data":{"trx":{ ...,"operations":[[66,{"fee":{"amount":100000,"asset_id":"1.3.0"},"owner_account":"1.2.3833","fund_id":"1.20.1","delta_amount":{"amount":2000000,"asset_id":"1.3.0"},"new_fee_rate":0, ...

Or (broadcast a signed transaction without explicit "new_fee_rate":null in the JSON)

{"id":15,"method":"call","params":[3,"broadcast_transaction",[{"ref_block_num":4708,"ref_block_prefix":3191402415,"expiration":"2021-11-03T23:04:42","operations":[[66,{"fee":{"amount":100000,"asset_id":"1.3.0"},"owner_account":"1.2.3833","fund_id":"1.20.1","delta_amount":{"amount":2000000,"asset_id":"1.3.0"},"extensions":[]}]],"extensions":[],"signatures":["1f38169815183c1e74766d54675222d9a6dd1b409a1f359fa60b7adb3c131e97292b49531a97161a515130f9e5cf56b70d6af42a5e82d63c84f51ad5abd29894c8"]}]]}

Actually, if broadcast the transaction to an API node with explicit "new_fee_rate":null in the JSON, the API node will accept the transaction, but the transaction will get rejected when being propagated to other nodes. If the API node is a block producer and the transaction is included in a block, the block will be rejected by other nodes because it can not pass Merkle tree validation.

The underlying issue is in FC. When unpacking (deserializing) an optional object, if the target variable is valid already, it will not get emptied / reset / overwritten. Related code: https://github.com/bitshares/bitshares-fc/blob/c72d5d31cc239a1a381cb3543402c9e2f4cc3f7f/include/fc/io/raw.hpp#L266-L273

    template<typename Stream, typename T>
    void unpack( Stream& s, fc::optional<T>& v, uint32_t _max_depth )
    { try {
       FC_ASSERT( _max_depth > 0 );
       --_max_depth;
       bool b; fc::raw::unpack( s, b, _max_depth );
       if( b ) { v = T(); fc::raw::unpack( s, *v, _max_depth ); }
    } FC_RETHROW_EXCEPTIONS( warn, "optional<${type}>", ("type",fc::get_typename<T>::name() ) ) }

Not sure if it's worth fixing. Usually we don't pass in a valid optional object when unpacking, although this time we did it.

[sonarcloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[abitmore]

This is actually a protocol change. If not coordinated properly, it may cause the chain to fork.

[abitmore]

Since it's not on mainnet yet, it only affects testnet. Will add hard fork protection code for testnet only.

[abitmore]

Actually the change is a soft fork. Won't add hard fork protection.