Vyper: ABI: consider using the "fixed" type for decimals

Created on 9 Dec 2017  路  41Comments  路  Source: vyperlang/vyper

Approved Discussion

Most helpful comment

@djrtwo master will produce the correct fixed168x10 will be bumping the version to 0.0.5 soon ;) will let you know on gitter.

All 41 comments

We were talking about this, but it is hard to verify formally. So we were reluctant.

I personally like being able to specify the decimal amount of the fixed point math instead of assuming 10 decimals. It needs to be decided what the maximum decimal points should be (18 is a good option)

I don't see a problem in changing the abi from decimals10 to fixed256x10 for compatibility while still having a fixed decimal amount for fixed point math for formal verification.

Is decimals10 224bits wide? I'd have assumed it occupies a full 256bit slot and reserves 10 decimal digits, ie. fixed256x10

@axic It is, thanks for the correcting this!

Decimals should be 128+34=162 bits wide, since ceil(log_2(1e10)) = 34. I think even 18 decimal places is only 60 extra bits, so 188 data bits total. The underlying data type is stored in a 256-bit word as you suggest because you can't have a partial word.

P.s. the maximum for decimal places is something like 38 because floor(log_10(2^128)) = 38

The format is fixed<M>x<N>, where M is the total number of bits occupied and N is the number of decimal digits, thus a value v is encoded as v * 10 ** N.

I thought 10 in decimal10 stands for 10 decimal places?

It does, the accuracy of decimal is 1/10000000000 which is 1e-10.

The bits reserved for the decimal part then should be ceil(log_2(1e10)) and if the maximum length for the integer part is 128 bits, then the right type is fixed168x10, given M must be divisible by 8.

This is a problem though since it is 6 bits larger than the underlying Viper type.

Hey folks. This is one of the python devs with the foundation. I'm currently trying to add both the Solidity fixed type and the Vyper decimal type to our ABI library: https://github.com/ethereum/eth-abi/pull/49

As people have noted, the fixed168x10 type is the correct equivalent but @axic said this is a problem. Can anyone give me an idea of how much of a problem this is or how exactly it might lead to issues? Currently, my plan is to just treat decimal values the same as fixed168x10 values but I'd like to know if I need to do any special bounds checking or overflow handling. Would love to hear people's insights into that.

I'm still not entirely sure how it is encoded.

This documentation suggests:

A value with a precision of 10 decimal places between -2^127 and (2^127 - 1).

Which matches with what @fubuloubu said:

Decimals should be 128+34=162 bits wide, since ceil(log_2(1e10)) = 34.

This in turn should mean that out of a 256 bit slot, it always uses 162 bits.

So I think the appropriate fixedMxN notation would be fixed162x10 if and only fixed would support any M, but it requires M to be divisible by 8, therefore the shortest type fitting is fixed168x10.

Now that means receiving data from a Vyper contract should not be a problem as long as the contract is trusted it will not return dirty bits on the top 6 bits.

I do not know however if Vyper does any cleanup and/or throws an error if dirty bits are sent to it.

@davesque in short I think your best choice is having a dedicated decimal10 type where the integer range is validated after ABI decoding/before ABI encoding it as fixed168x10 since your library supports the latter.

A good alternative would be Vyper adopting fixed168x10 馃槈

@axic I'll do what you're suggesting and just implement some manual, strict bounds checking to ensure that values are exactly between and including -2^127 and 2^127 - 1.

Actually, I don't think it would matter if the Solidity fixed type supported bit widths not congruent with 0 modulo 8 since log2((2^128 - 1) * 10^10 + 1) < 1621. So you'd still need to manually enforce some bounds since 162 bits don't exactly represent the number of distinct values in the decimal range.

1: I'm using this value since (2^127 - 1) * 10^10 + 2^127 * 10^10 + 1 is the true number of distinct values between and including -2^127 and 2^127 - 1 with 10 places of decimal precision. And that value is equal to (2^128 - 1) * 10^10 + 1.

I wonder if it would be clearer if the first number represented the size of the integer portion (e.g. 128 bits) and the second number represented the number of decimals the non-integer portion takes, (1e10 takes 34 bits minimum, so 40 bits to put it on an boundary of 8).

Another note is that this format could do a different integer type for the base, but never does in practice. Maybe we can just drop the number of bits and assume 128 + log2(numDecimals) rounded to every 8 e.g. fixed10

Honestly, I like the way the fixed type works right now. It's explicit and makes it clear how many distinct values can be represented which is probably the most important thing to keep in mind other than the decimal precision. That kind of information would be obscured if the numbers in the type had a different meaning.

The motivation for M is define the total size of the value on the stack item. This is consistent with every other type (i.e. int128, uint256, etc.) Making it byte aligned makes it super simple for all the tools to properly clean the values. It is a tad bit harder to do that with decimal.

A good alternative would be Vyper adopting fixed168x10 :wink:

This seems like the best option. What would it take to make that happen?

Is there any internal Vyper consensus for dropping decimal in favor of fixedMxN? I'm trying to gauge if this is still in early discussions or might we see a shift/adoption sooner rather than later?

Casper uses decimal10 and needs support added asap to web3.py. We're down to go the fixed168x10 route but need to get some clarity so we can make some decisions.

@jacqueswww I think for compatibility, I'm for it

Internally Vyper can still offer the same range decimal10 does, it just needs to clean up the incoming data:

  • either silently truncate
  • or raise a failure if there is an overflow

If it uses the same range, there is no need to anything on the output side as it will always fit into fixed168x10.

it just needs to clean up the incoming data:

Sure, but why commit to adding overhead for inbound data conversions? If the original use cases of decimal10 are flexible, it seems cheaper to change the internal Vyper definition now than it is to pay overhead, roughly forever.

To met it seems that Vyper has three number types: int128, uint256 and decimal10. Conversion between int128 and decimal10 is nice and easy, it is only about dropping/adding the decimal digits as the value range for the integer part is the very same for both.

It probably makes good sense keeping this and a good way to have the best of both worlds is by validating incoming external data and leaving the internal calculations "cheap".

As I understand the current definition of decimal10, this is the largest valid value:

(2**127 * 10**10 - 1) / 10**10 => 170141183460469231731687303715884105727.9999999999

and this value would be invalid:

2**127 => 170141183460469231731687303715884105728.0000000000


The presumed representation of those numbers in hex-encoded binary would be:

valid:
0x12a05f1ffffffffffffffffffffffffffffffffff (21 bytes, 168 bits)

and invalid:
0x12a05f20000000000000000000000000000000000 (21 bytes, 168 bits)


As far as I know, decimal10 would become the only ABI type that requires validation by comparing to a hard-coded maximum and minimum value. The other fixed-size types are all valid for any value that can be represented in the given number of bytes (except bool which is trivial to validate as 0 or 1).

Also, what does it do if someone sends in 2**127? Does it simulate an overflow and wrap around to the most negative value? Or does it apply a ceiling and give you the maximum valid value? I'm not sure. We don't have to answer this question if we use fixed168x10.

@carver Agree. And my understanding is that the bounds are defined as such: [-2^127, 2^127 - 1]

So that would be [-170141183460469231731687303715884105728.0000000000, 170141183460469231731687303715884105727.0000000000].

My rational for this comes from taking as likely an interpretation as possible from the Vyper documentation that corresponds with the usual behavior of a two's complement integer encoded in 128 bits: http://viper.readthedocs.io/en/latest/types.html#decimals

@davesque yes, that's right. The max would be 2**127-1, truncating the decimal portion, otherwise it would have incorrect behavior as far as round is concerned if we let the decimal portion go above that.

Is there an update on the status of decimal10? Is support to be dropped in favor of fixedMxN?

I'm currently working on a wrapper for the casper contract in Parity so also keen to know what the decision is here

To justify a new decimal10 ABI type, I propose that we would need to come up with a use case in which the extreme values* from fixed168x10 must not be valid. I haven't come up with one yet, or seen one. If this conversation stalls, the default should be to use the existing ABI type: fixed168x10.

*specifically the 10-digit decimals exclusively between (2**127 - 1, 2**168 / 10**10) == (170141183460469231731687303715884105727.0000000000, 37414441915671114706014331717536845303191.8731001856), and the complementary negative numbers

This is why we need the ABI spec to be a separate repo lol

I will make this change now, I believe we converged on just changing it on the ABI output level to just state fixed168x10.

Cool :+1:

For what it's worth, I'm not sure how much discussion was had on the byte size itself: fixed256x10 could be just as reasonable a choice. But the impact of that decision seems to affect Vyper more internally than externally, so I'm going to bow out of the discussion.

Thanks everyone for taking the time to talk through all of this.

For what it's worth, I'm not sure how much discussion was had on the byte size itself: fixed256x10 could be just as reasonable a choice.

I had the same feeling on that. Since the true bounds of decimal10 (which are [-2^127, 2^127 - 1]) are already not honored by the fixed168x10 type, would it make sense to just use fixed256x10 as @DavidKnott suggested earlier in this thread? Is there any reason to artificially cap the possible values to 168 bits?

I think in any case Vyper should throw if the value is out of range.

If that is the case then probably fixed168x10 is less misleading, because it doesn't suggest such a large domain for values (however this is a very slim reasoning).

@davesque 168 is because that's the max numbers of bits that can get flipped. It still takes up 256 bits.

Checking in on the status of this one. Any eta? It's a blocker various casper dev

@djrtwo unfortunately pyethereum fails if I do the conversion, so we have to move to py-evm first.
I am starting on https://github.com/ethereum/vyper/issues/652 this week.

oh no! That's the reason we want this resolved because we want to get out of pyethereum and onto pyevm/web3.py

@djrtwo Hehehe, I am busy with the conversion right now. So hoping sometimes this week we have something resembling a vyper test suite that doesn't break ;)

Edit:
Progress so far:

59 failed, 539 passed, 22 warnings, 131 error in 27.08 seconds 

Thanks! very excited for us all to be off pyethereum...

@djrtwo master will produce the correct fixed168x10 will be bumping the version to 0.0.5 soon ;) will let you know on gitter.

Very exciting @jacqueswww
I'll start doing a migration with master and plug in 0.0.5 when its ready.

@jacqueswww was this change made in the end?

@axic ha! only saw your comment now. Yes the chagne has been made. Closing issue.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

fubuloubu picture fubuloubu  路  3Comments

denis-bogdanas picture denis-bogdanas  路  3Comments

fubuloubu picture fubuloubu  路  3Comments

lsaether picture lsaether  路  4Comments

domrany64 picture domrany64  路  3Comments