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

Increase coverage #149

Merged
merged 18 commits into from
Dec 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions packages/contracts/.solcover.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
const { accountsList: accounts } = require("./buidlerAccountsList2k.js");
const { accountsList } = require("./buidlerAccountsList2k.js");
// syntax for solcover network (ganache based) is different:
// https://hardhat.org/plugins/solidity-coverage.html#configuration
// Link in providerOptions:
// https://github.com/trufflesuite/ganache-core#options
const accounts = accountsList.map(a => ({ secretKey: a.privateKey, balance: '0xc097ce7bc90715b34b9f1000000000' }))

module.exports = {
providerOptions: {
Expand All @@ -7,10 +12,8 @@ module.exports = {

// Improve performance by skipping statements and functions. Tool still checks lines of code and branches:
// https://github.com/sc-forks/solidity-coverage/blob/master/docs/advanced.md
measureStatementCoverage: false,
measureFunctionCoverage: false,

testfiles: "test/LUSDTokenTest.js",
//measureStatementCoverage: false,
//measureFunctionCoverage: false,

skipFiles: [
"TestContracts/",
Expand All @@ -19,6 +22,7 @@ module.exports = {
"Interfaces/",
"Dependencies/Context.sol",
"Dependencies/IERC20.sol",
"Dependencies/IERC2612.sol",
"Dependencies/Math.sol",
"Dependencies/Ownable.sol",
"Dependencies/SafeMath.sol",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,23 @@ contract LUSDTokenTester is LUSDToken {
_approve(owner, spender, amount);
}

function getChainId() external pure returns (uint256 chainID) {
//return _chainID(); // it’s private
assembly {
chainID := chainid()
}
}

function getDigest(address owner, address spender, uint amount, uint nonce, uint deadline) external view returns (bytes32) {
return keccak256(abi.encodePacked(
uint16(0x1901),
domainSeparator(),
keccak256(abi.encode(_PERMIT_TYPEHASH, owner, spender, amount, nonce, deadline))
)
);
}

function recoverAddress(bytes32 digest, uint8 v, bytes32 r, bytes32 s) external pure returns (address) {
return ecrecover(digest, v, r, s);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// SPDX-License-Identifier: MIT

pragma solidity 0.6.11;

import "../Dependencies/LiquitySafeMath128.sol";

/* Tester contract for math functions in LiquitySafeMath128.sol library. */

contract LiquitySafeMath128Tester {
using LiquitySafeMath128 for uint128;

function add(uint128 a, uint128 b) external pure returns (uint128) {
return a.add(b);
}

function sub(uint128 a, uint128 b) external pure returns (uint128) {
return a.sub(b);
}
}
11 changes: 10 additions & 1 deletion packages/contracts/contracts/TestContracts/NonPayable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,23 @@

pragma solidity 0.6.11;

//import "../Dependencies/console.sol";


contract NonPayable {
bool isPayable;

function setPayable(bool _isPayable) external {
isPayable = _isPayable;
}

function forward(address _dest, bytes calldata _data) external payable {
(bool success, bytes memory returnData) = _dest.call{ value: msg.value }(_data);
//console.logBytes(returnData);
require(success, string(returnData));
}

receive() external payable {
revert();
require(isPayable);
}
}
30 changes: 30 additions & 0 deletions packages/contracts/contracts/TestContracts/SortedTrovesTester.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// SPDX-License-Identifier: MIT

pragma solidity 0.6.11;

import "../Interfaces/ISortedTroves.sol";


contract SortedTrovesTester {
ISortedTroves sortedTroves;

function setSortedTroves(address _sortedTrovesAddress) external {
sortedTroves = ISortedTroves(_sortedTrovesAddress);
}

function insert(address _id, uint256 _ICR, uint _price, address _prevId, address _nextId) external {
sortedTroves.insert(_id, _ICR, _price, _prevId, _nextId);
}

function remove(address _id) external {
sortedTroves.remove(_id);
}

function reInsert(address _id, uint256 _newICR, uint _price, address _prevId, address _nextId) external {
sortedTroves.reInsert(_id, _newICR, _price, _prevId, _nextId);
}

function getCurrentICR(address, uint) external pure returns (uint) {
return 1;
}
}
29 changes: 29 additions & 0 deletions packages/contracts/test/DefaultPoolTest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
const testHelpers = require("../utils/testHelpers.js")
const DefaultPool = artifacts.require("./DefaultPool.sol");
const NonPayable = artifacts.require('NonPayable.sol')

const th = testHelpers.TestHelper
const dec = th.dec

contract('DefaultPool', async accounts => {
let defaultPool
let nonPayable

let [owner] = accounts

beforeEach('Deploy contracts', async () => {
defaultPool = await DefaultPool.new()
nonPayable = await NonPayable.new()
await defaultPool.setAddresses(owner, owner)
})

it('sendETH(): fails if receiver cannot receive ETH', async () => {
const amount = dec(1, 'ether')

await web3.eth.sendTransaction({ to: defaultPool.address, from: owner, value: amount })

await th.assertRevert(defaultPool.sendETH(nonPayable.address, amount, { from: owner }), 'DefaultPool: sending ETH failed')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm does this revert because defaultPool.sendETH is only callable TroveManager though? If so, it wouldn't hit the final _require(success, ...) that we want to test here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, but no, because owner is the TroveManager here 😉 :
https://github.com/liquity/dev/pull/149/files#diff-ff756b67b4b4fded33dc971414822ef458d9dc958a1ee0bb38f11c99d6fc4daaR17

Anyway, this highlights again the importance of revert messages. While testing these PRs I restore the message check in testHelpers, but I may forget it sometimes. Hopefully we can have issue #99 fixed soon! 🤞

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, I see. Nice.

Definitely, thanks!

})
})

contract('Reset chain state', async accounts => { })
98 changes: 83 additions & 15 deletions packages/contracts/test/LUSDTokenTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,14 @@ contract('LUSDToken', async accounts => {

describe('Basic token functions', async () => {
beforeEach(async () => {
chainId = await web3.eth.getChainId()

const contracts = await deploymentHelper.deployTesterContractsBuidler()

lusdTokenTester = contracts.lusdToken
// for some reason this doesn’t work with coverage network
//chainId = await web3.eth.getChainId()
chainId = await lusdTokenTester.getChainId()

stabilityPool = contracts.stabilityPool
troveManager = contracts.stabilityPool
borrowerOperations = contracts.borrowerOperations
Expand Down Expand Up @@ -280,7 +283,21 @@ contract('LUSDToken', async accounts => {
await assertRevert(lusdTokenTester.transfer(stabilityPool.address, 1, { from: alice }))
await assertRevert(lusdTokenTester.transfer(borrowerOperations.address, 1, { from: alice }))
})


it('decreaseAllowance(): decreases allowance by the expected amount', async () => {
await lusdTokenTester.approve(bob, dec(3, 18), { from: alice })
assert.equal((await lusdTokenTester.allowance(alice, bob)).toString(), dec(3, 18))
await lusdTokenTester.decreaseAllowance(bob, dec(1, 18), { from: alice })
assert.equal((await lusdTokenTester.allowance(alice, bob)).toString(), dec(2, 18))
})

it('decreaseAllowance(): fails trying to decrease more than previously allowed', async () => {
await lusdTokenTester.approve(bob, dec(3, 18), { from: alice })
assert.equal((await lusdTokenTester.allowance(alice, bob)).toString(), dec(3, 18))
await assertRevert(lusdTokenTester.decreaseAllowance(bob, dec(4, 18), { from: alice }), 'ERC20: decreased allowance below zero')
assert.equal((await lusdTokenTester.allowance(alice, bob)).toString(), dec(3, 18))
})

// EIP2612 tests

it('Initializes PERMIT_TYPEHASH correctly', async () => {
Expand All @@ -296,32 +313,40 @@ contract('LUSDToken', async accounts => {
assert.equal(toBN(await lusdTokenTester.nonces(alice)).toString(), '0');
});

it('permits and emits an Approval event (replay protected)', async () => {
// Create the approval tx data
const approve = {
owner: alice,
spender: bob,
value: 1,
}

const deadline = 100000000000000
// Create the approval tx data
const approve = {
owner: alice,
spender: bob,
value: 1,
}

const buildPermitTx = async (deadline) => {
const nonce = await lusdTokenTester.nonces(approve.owner)

// Get the EIP712 digest
const digest = getPermitDigest(
tokenName, lusdTokenTester.address,
chainId, tokenVersion,
approve.owner, approve.spender,
approve.value, nonce, deadline
)

const { v, r, s } = sign(digest, alicePrivateKey)

// Approve it
const receipt = await lusdTokenTester.permit(
const tx = lusdTokenTester.permit(
approve.owner, approve.spender, approve.value,
deadline, v, hexlify(r), hexlify(s)
)

return { v, r, s, tx }
}

it('permits and emits an Approval event (replay protected)', async () => {
const deadline = 100000000000000

// Approve it
const { v, r, s, tx } = await buildPermitTx(deadline)
const receipt = await tx
const event = receipt.logs[0]

// Check that approval was successful
Expand All @@ -338,6 +363,49 @@ contract('LUSDToken', async accounts => {
assertRevert(lusdTokenTester.permit('0x0000000000000000000000000000000000000000',
approve.spender, approve.value, deadline, '0x99', r, s), 'LUSD: Recovered address from the sig is not the owner')
})

it('permits and emits an Approval event (replay protected), with infinite deadline', async () => {
const deadline = 0

// Approve it
const { v, r, s, tx } = await buildPermitTx(deadline)
const receipt = await tx
const event = receipt.logs[0]

// Check that approval was successful
assert.equal(event.event, 'Approval')
assert.equal(await lusdTokenTester.nonces(approve.owner), 1)
assert.equal(await lusdTokenTester.allowance(approve.owner, approve.spender), approve.value)

// Check that we can not use re-use the same signature, since the user's nonce has been incremented (replay protection)
assertRevert(lusdTokenTester.permit(
approve.owner, approve.spender, approve.value,
deadline, v, r, s), 'LUSD: Recovered address from the sig is not the owner')

// Check that the zero address fails
assertRevert(lusdTokenTester.permit('0x0000000000000000000000000000000000000000',
approve.spender, approve.value, deadline, '0x99', r, s), 'LUSD: Recovered address from the sig is not the owner')
})

it('permits(): fails with expired deadline', async () => {
const deadline = 1

const { v, r, s, tx } = await buildPermitTx(deadline)
await assertRevert(tx, 'LUSD: Signature has expired')
})

it('permits(): fails with the wrong signature', async () => {
const deadline = 100000000000000

const { v, r, s } = await buildPermitTx(deadline)

const tx = lusdTokenTester.permit(
carol, approve.spender, approve.value,
deadline, v, hexlify(r), hexlify(s)
)

await assertRevert(tx, 'LUSD: Recovered address from the sig is not the owner')
})
})
})

Expand Down
21 changes: 21 additions & 0 deletions packages/contracts/test/LiquitySafeMath128Test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
const testHelpers = require("../utils/testHelpers.js")
const th = testHelpers.TestHelper

const LiquitySafeMath128Tester = artifacts.require("LiquitySafeMath128Tester")

contract('LiquitySafeMath128Tester', async accounts => {
let mathTester

beforeEach(async () => {
mathTester = await LiquitySafeMath128Tester.new()
})

it('add(): reverts if overflows', async () => {
const MAX_UINT_128 = th.toBN(2).pow(th.toBN(128)).sub(th.toBN(1))
await th.assertRevert(mathTester.add(MAX_UINT_128, 1), 'LiquitySafeMath128: addition overflow')
})

it('sub(): reverts if underflows', async () => {
await th.assertRevert(mathTester.sub(1, 2), 'LiquitySafeMath128: subtraction overflow')
})
})
Loading