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

SafeMath for uint32 and all other variations #1625

Closed
cedricwalter opened this issue Jan 28, 2019 · 12 comments
Closed

SafeMath for uint32 and all other variations #1625

cedricwalter opened this issue Jan 28, 2019 · 12 comments
Labels
contracts Smart contract code. needs milestone Interesting features or improvements that are not yet assigned to a milestone.

Comments

@cedricwalter
Copy link

🧐 Motivation

📝 Details
I need safe math operations on uint32, i could do a PR but there is a lot of type to support (uint8 to uint256 in steps of 8 (unsigned of 8 up to 256 bits) ) and before jumping in, i want to know what this PR could/should contains

@frangio
Copy link
Contributor

frangio commented Jan 28, 2019

Hi @cedricwalter. Thank you for opening an issue prior to contributing a PR. 😊 Please see my response here: #1484 (comment).

Unfortunately we've decided in the past to only support 256 bit arithmetic. Since that is the native word size in the EVM and everything else is emulated by Solidity, we don't see any situation where 32 bit arithmetic should be used.

Could you share your motivation for working with 32 bit integers? If it involves gas costs, please include code to measure them. Given that Solidity emulates 32 bit arithmetic with masks, shifting, and "manual" overflow detection, I'm not convinced that using 32 bit integers is actually cheaper than integers of the native size (256 bit).

@ackintosh
Copy link

Hi. 😊

I've noticed that the types effects to gas costs in case of numbers in inside of structs. 💡

Details are in the CryptZombies tutorial below:
https://cryptozombies.io/en/lesson/3/chapter/4

Struct packing to save gas

If you have multiple uints inside a struct, using a smaller-sized uint when possible will allow Solidity to pack these variables together to take up less storage.


With the sample code below, I've measured that how much of gas cost difference between uint(uint256) and uint8 which are exists inside of struct.

Sample code (GasCosts.sol)

pragma solidity >=0.4.21 <0.6.0;

contract GasCosts {
    StructA public structA;
    struct StructA {
        uint a;
        uint b;
        uint c;
    }

    StructB public structB;
    struct StructB {
        uint8 a;
        uint8 b;
        uint8 c;
    }

    function saveStructAtoStorage() public {
        structA = StructA(1, 2, 3);
    }

    function saveStructBtoStorage() public {
        structB = StructB(1, 2, 3);
    }
}

Operations on truffle console

StructA (uint)

  • operation
GasCosts.deployed().then(function(instance) { instance.saveStructAtoStorage() });
  • log on the operation (truffle develop --log)
  develop:ganache eth_getBlockByNumber +45s
  develop:ganache eth_getBlockByNumber +7ms
  develop:ganache eth_sendTransaction +4ms
  develop:ganache  +33ms
  develop:ganache   Transaction: 0x69f6b8ec2e1be68117f05adcab1562e04a8e73c0a43bf6182ecc75c891ee5e16 +0ms
  develop:ganache   Gas usage: 81602 +0ms
  develop:ganache   Block Number: 33 +0ms
  develop:ganache   Block Time: Sat Feb 16 2019 14:23:05 GMT+0900 (日本標準時) +1ms
  develop:ganache  +0ms
  develop:ganache eth_getTransactionReceipt +0ms

Gas usage: 81602

StructB (uint8)

  • operation
GasCosts.deployed().then(function(instance) { instance.saveStructBtoStorage() });
  • logs on the operation (truffle develop --log)
  develop:ganache eth_getBlockByNumber +36s
  develop:ganache eth_getBlockByNumber +6ms
  develop:ganache eth_sendTransaction +4ms
  develop:ganache  +32ms
  develop:ganache   Transaction: 0x762db0c9a598d60fbdd81777fc3877bbfbaad2536e9badb27b5d47a1d9f5f688 +0ms
  develop:ganache   Gas usage: 52481 +0ms
  develop:ganache   Block Number: 34 +0ms
  develop:ganache   Block Time: Sat Feb 16 2019 14:23:41 GMT+0900 (日本標準時) +1ms
  develop:ganache  +0ms
  develop:ganache eth_getTransactionReceipt +0ms

Gas usage: 52481


From the results, in case of inside of struct, there's a benefit that using the subtypes instead of uint. 💡

@wbt
Copy link

wbt commented Feb 21, 2019

Unfortunately we've decided in the past to only support 256 bit arithmetic. Since that is the native word size in the EVM and everything else is emulated by Solidity, we don't see any situation where 32 bit arithmetic should be used.

Could you share your motivation for working with 32 bit integers? If it involves gas costs, please include code to measure them. Given that Solidity emulates 32 bit arithmetic with masks, shifting, and "manual" overflow detection, I'm not convinced that using 32 bit integers is actually cheaper than integers of the native size (256 bit).

This repository also demonstrates measurable savings in gas costs, showing that the use of 256-bit arithmetic adds an inefficiency cost of about 1/3 compared to using 32-bit integers, in a setting where 32-bit integers suffice. To reproduce, clone that repo and run truffle migrate as noted in its readme.

@nventuro
Copy link
Contributor

Hm, I wonder how those numbers would change in more dynamic scenarios (e.g. the values are received as parameters instead of being hardcoded, and some math is done on them).

@nventuro nventuro added this to the v2.3 milestone Feb 22, 2019
@nventuro nventuro added research contracts Smart contract code. labels Feb 22, 2019
@nventuro
Copy link
Contributor

Tagging this for 2.3 so that we keep track of it and continue the discussion, but this doesn't necessarily mean that we will include support in that version.

@wbt
Copy link

wbt commented Feb 22, 2019

Hm, I wonder how those numbers would change in more dynamic scenarios (e.g. the values are received as parameters instead of being hardcoded, and some math is done on them).

@nventuro I'm not sure what you mean. In the example repo the uint values are received by EVM as parameters; the actual values being stored are randomly generated in Javascript.
The number of bits in the integer representation is hard-coded, as usual.

@nventuro
Copy link
Contributor

Sorry @wbt, I had only looked at the example @ackintosh provided. Your example should indeed trigger some of Solidity's masking code, since you're both storing packed uint32 values, and emitting events with uint32 values.

That said, it does seem to serve as a good estimate of an upper bound in gas savings, since it compares four values packed into a single slot as opposed to four full slots, with functions that only read/write said slots.

@wbt
Copy link

wbt commented Feb 22, 2019

@nventuro Yes, I believe packing is the primary gas-savings motivation for using smaller ints.
In some cases, there are also concerns about interoperability with legacy systems that don't support such large numbers, but the gas savings from tight packing is a stronger and more general motivator.

@wbt
Copy link

wbt commented Feb 28, 2019

I've added a function "turnUpHeat" in the example repo which does some math on what the contract is receiving as dynamic variables.

Also, I think you'd have to be using uint8s where there is more frequent changing of values to get closer to a good estimate of an upper bound in gas savings. Still, I don't think we'd need to approach the upper bound in gas savings when an even lower savings level is still significant, and (in my mind) justifies the relatively low cost of adapting SafeMath to smaller integer types.

@nventuro
Copy link
Contributor

nventuro commented Mar 7, 2019

@wbt I played around with your migrations script for a bit, adding calls to turnUpHeat, using new functions to use more/less storage slots, other arithmetic functions, etc., and found that the gas cost difference is very much dependent on actual usage of storage - in some scenarios, the extra overhead of 32-byte arithmetic outweighs the benefit of packed structs. This means that each developer should analyse gas for their particular project and decide which integer size to use based on estimated usage - no simple rule of thumb can be given.

That said, there are cases where savings can be quite large (~30%), so providing support may make sense. My main issue is regarding a lack of language support in this regards (no generics, in particular): we'd have to maintain multiple SafeMathNBytes contracts with extremely similar content, or use some sort of code autogeneration tool to take care of this for us. @frangio thoughts?

@wbt
Copy link

wbt commented Mar 8, 2019

each developer should analyse gas for their particular project and decide which integer size to use based on estimated usage - no simple rule of thumb can be given.

I agree, which is why "everybody always use uint256 or don't use OpenZeppelin's safemath" doesn't seem appropriate.

Code autogeneration seems like it'd be relatively easy, or copy-paste-modify for those where it's requested (or maybe starting with the powers of 2, 8-256). I would recommend dropping "Bytes" from the suggested name, and have N match the bit count (e. g. "SafeMath32"), so the naming convention matches how uints are named.

@frangio frangio removed this from the v2.3 milestone Apr 8, 2019
@frangio frangio added the needs milestone Interesting features or improvements that are not yet assigned to a milestone. label Apr 8, 2019
@nventuro nventuro removed the research label Apr 8, 2019
@nventuro
Copy link
Contributor

#1926 added SafeCast, allowing SafeMath to be used on smaller types by:

  1. using regular 256 bit SafeMath on the smaller types, which will upcast them, and then
  2. downcasting with SafeCast

Arithmetic will be performed on 256 types (which is often useful since it allows for intermediate steps in a calculation to go over smaller types, e.g. when multiplying multiple numbers before dividing), and the final downcast will assert that the result fits in the intended type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contracts Smart contract code. needs milestone Interesting features or improvements that are not yet assigned to a milestone.
Projects
None yet
Development

No branches or pull requests

5 participants