ds-math icon indicating copy to clipboard operation
ds-math copied to clipboard

Make this into a library

Open dmdque opened this issue 6 years ago • 7 comments

It would be helpful if this were made into a library using Solidity's library.

dmdque avatar Mar 24 '18 00:03 dmdque

I need this also and as far as I know it's also not in openzeppelin (just SafeMath which is less than this) so I want to contribute this (see Pullrequest linked above). Please admin like @rainbreak review and approve (personal mention because in this repo seems to be little activity...)

JohannesMayerConda avatar Nov 29 '18 17:11 JohannesMayerConda

Using this as a library would mean having the cost of DELEGATECALL (700 gas) on every arithmetic operation. Have you considered this?

[not necessarily true, see below]

rainbreak avatar Nov 30 '18 02:11 rainbreak

That's right (a test showed me gasUsed can be in practice even higher). Out of my experience the gas cost factor is less critical than getting all the code you need into a contract without running into a block gas limit reached error (at least upon a certain point of complexity). That's where a library can help. It's not uncommon to put math stuff into a library e.g. openzeppelin math libraries. I guess everyone who want's to optimize after completing development could still copy the functions he needs from the library.

But I can see your point. Should I copy&paste the library code into a DSMathC.sol (C for contract L for library)? Then everyone can decide. Downside is that two files have to be maintained

I especially neeeded the wmul() func and for me the best thing would be to have it in openzeppelin. I hope this can be a next step...

JohannesMayerConda avatar Nov 30 '18 08:11 JohannesMayerConda

You have a good point about the cost of DELEGATECALL. As @JohannesMayerConda alluded to, more complex projects can't fit all functionality into a single contract due to the block gas limit, and have to be split up into multiple contracts. Having each one inherit ds-math is cumbersome.

Having the option for both seems like a no-brainer.

dmdque avatar Dec 01 '18 05:12 dmdque

I didn't realize that ds-math is part of DappHub and tests can be executed via https://dapp.tools/ (see PullRequest discussion). My PullRequest would change this library into a truffle project which doesn't fit into the system and was therefore rejected (which I totally understand). I don't know if this would be an easy change to fit into the system but as @rainbreak said gas would also increase and isn't desired.

I feel like adapting my Pullrequest to adding a library containing duplicate code that is not even being used (because of higher gas) feels wrong so I suggest we close this issue (by an admin) and everyone interested in using ds-math as library approach can checkout my forked implementation via https://github.com/JohannesMayerConda/ds-math. I've adapted the readme to reflect this. I think a separate repository using truffle makes sense anyway for being closer to openzeppelin SafeMath Library

JohannesMayerConda avatar Dec 03 '18 12:12 JohannesMayerConda

FYI most reputable tokens and protocols use OpenZeppelin's SafeMath, which is a library. The cost of gas is deemed acceptable for most projects out there.

dmdque avatar Dec 03 '18 22:12 dmdque

I was incorrect in the above comment about delegatecall being used everywhere. This is only true for public functions. However the functions in ds-math are all internal, so a user of a ds-math library would just be inlining them and accessing them with jump.

This removes the concern about excess runtime gas costs, but also the concern about deploytime gas costs.

https://solidity.readthedocs.io/en/v0.5.1/contracts.html#libraries

Libraries can be seen as implicit base contracts of the contracts that use them. They will not be explicitly visible in the inheritance hierarchy, but calls to library functions look just like calls to functions of explicit base contracts (L.f() if L is the name of the library). Furthermore, internal functions of libraries are visible in all contracts, just as if the library were a base contract. Of course, calls to internal functions use the internal calling convention, which means that all internal types can be passed and types stored in memory will be passed by reference and not copied. To realize this in the EVM, code of internal library functions and all functions called from therein will at compile time be pulled into the calling contract, and a regular JUMP call will be used instead of a DELEGATECALL.

rainbreak avatar Dec 07 '18 04:12 rainbreak