search

wharfkit / antelope

Core types, client interfaces, and other tools for working with Antelope-based blockchains.
Other
44 stars 23 forks source link

Fix to allow UInt64.from(0) for an Asset #45

Closed aaroncox closed 1 year ago

aaroncox commented 1 year ago

This is apparently a valid value coming from nodeos.

I hate having to do this, but in order to properly serialize this table row from its (seemingly) valid ABI, we need to allow new Asset(UInt64.from(0)) to allow deserializing the voters table.

See the tests for what was failing when using the actual ABI from the system contract.

aaroncox commented 1 year ago

I'm going to publish this as @greymass/eosio@next (the next tag) to see if it fixes the issue for those who reported it.

aaroncox commented 1 year ago

Just committed https://github.com/greymass/eosio-core/pull/45/commits/c21c0d19fcf858373a51ae75c147e85664c51b6b which is a new failing unit test I'd expect to work.

Either the change I made doesn't fix the root of this problem, or another fix needs to be made.

What I did was went and queried the eosio contract's voter table with json: false to get the hex data for the row, and this is what nodeos provided:

80b1915e5d268dca000000000000000000d54af8550100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

The fix I made in the PR does successfully decode the row.

However when I take the same row data and serialize it again, this is the result:

80b1915e5d268dca000000000000000000d54af855010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

It's missing 0000000000000000 somewhere in there, which I can only assume is data that's missing because the serializer is not encoding that empty symbol back out as hex.

Is that OK that the data is shorter? Should we be padding some 0's somewhere? Is there a change to the asset/symbol/serializer we need to make so that data is accounted for?

jnordberg commented 1 year ago

That's going to blow up if ever used in a transaction, not likely but we should fix it to behave the same as nodeos. I'll take a look in the c++ sources and check how they treat a "null" asset.