poanetwork / posdao-contracts

Smart contracts for POSDAO (Proof of Stake Decentralized Autonomous Organization consensus), a DPOS consensus implemented in Solidity and running within EVM with swappable BFT consensus
Other
106 stars 50 forks source link

Add Solidity unit tests for SafeMath library #4

Open vbaranov opened 5 years ago

vbaranov commented 5 years ago

Child of #1

vbaranov commented 5 years ago

@varasev btw, why not the Open Zeppelin's implementation of Safemath lib is used? At least, it is not the latest one.

varasev commented 5 years ago

@vbaranov I took the implementation from our consensus contracts and didn't check if Open Zeppelin contained a new version. Thanks for reminding - I'll update that.

varasev commented 5 years ago

SafeMath was updated in https://github.com/poanetwork/pos-contracts/commit/d7c14b252456c45bc8fe2894c879367efac631bc.

DemiMarie commented 5 years ago

Can this be closed? The OpenZeppelin implementation is heavily tested, so testing it further would not be a good use of our efforts. (It would not be pointless ― just a poor use of POA Network resources.)

DemiMarie commented 5 years ago

@varasev @vbaranov

vbaranov commented 5 years ago

@DemiMarie

The OpenZeppelin implementation is heavily tested

That's true.

It would not be pointless ― just a poor use of POA Network resources.

I agree with this too. And we don't need to invent a bicycle. We can get those tests to our repo.

I think that every contract in the dPoS set should be covered with unit tests. There are two ways to add SafeMath lib tests: 1) or we can add SafeMathLib by adding openzeppelin-solidity npm package (thus, we will automatically get the unit tests for it) 2) or we can copy SafeMathLib unit tests to our repo.

In both cases we don't need so much resources.

@DemiMarie @varasev which one do you prefer (if any)?

varasev commented 5 years ago

I'm working on unit tests. Let me do this because I'm the only developer at the moment who designed and wrote each function of pos contracts from scratch, there are many nuances and details that are known only to me.

As of SafeMath lib - I'll copy them from openzeppelin-solidity.

DemiMarie commented 5 years ago

@varasev status?

varasev commented 5 years ago

Isn't done yet. I was busy doing the changes in the contracts, reviewing White Paper and design layouts.