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

Move contracts to subdirectories #1253

Merged
merged 5 commits into from
Sep 3, 2018
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
31 changes: 0 additions & 31 deletions contracts/LimitBalance.sol

This file was deleted.

4 changes: 2 additions & 2 deletions contracts/access/SignatureBouncer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ pragma solidity ^0.4.24;

import "../ownership/Ownable.sol";
import "../access/rbac/RBAC.sol";
import "../ECRecovery.sol";
import "../cryptography/ECDSA.sol";


/**
Expand Down Expand Up @@ -30,7 +30,7 @@ import "../ECRecovery.sol";
* much more complex. See https://ethereum.stackexchange.com/a/50616 for more details.
*/
contract SignatureBouncer is Ownable, RBAC {
using ECRecovery for bytes32;
using ECDSA for bytes32;

string public constant ROLE_BOUNCER = "bouncer";
uint internal constant METHOD_ID_SIZE = 4;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
pragma solidity ^0.4.24;


import "./payment/PullPayment.sol";
import "./lifecycle/Destructible.sol";
import "../payment/PullPayment.sol";
import "../lifecycle/Destructible.sol";


/**
* @title Bounty
* @title BreakInvariantBounty
* @dev This bounty will pay out to a researcher if they break invariant logic of the contract.
*/
contract Bounty is PullPayment, Destructible {
contract BreakInvariantBounty is PullPayment, Destructible {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain the reasoning behind this rename? I'm not sure I prefer the new name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because now the directory is called bounties and it has room for other types of bounties, we need a less generic name. I used the docstring to come up with this name:
"This bounty will pay out to a researcher if they break invariant logic of the contract."

The alternative we had documented on the issue #1177 was to move it to examples, but we didn't document any other option for the name. Do you have a better name in mind, or would you like a different solution for taking it out of the root dir?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I remember that conversation now. The problem is that I can't imagine other types of bounties possible. Do you have any in mind?

Copy link
Contributor Author

@come-maiz come-maiz Aug 30, 2018

Choose a reason for hiding this comment

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

More or less. I might be stretching this a little, but there could be an oracle that checks who closed a github bug, and then let that person claim the bounty. Following the same idea, a bounty on ZeppelinOS might be totally on chain, but not just for breaking a contract but for publishing a new version or something. I think we will see more things related to bounties in the future, and they can all fit in this directory. But these are just predictions, I don't have a strong opinion about this directory. This was jsut the best option I could come up with, but I'm happy moving the contract somewhere else, giving it a different name, or dropping it.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a super cool idea, but if it's ever developed I don't see it being a part of OpenZeppelin because it would be a complex system.

Copy link
Contributor

Choose a reason for hiding this comment

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

An OracleBounty sounds like a nice and straightforward usage example of an Oracle interface, once that's finalized (#971), I'd like to see such a contract. However, I don't think we'd also want to have an in-house Oracle implementation, so I'm not sure if this kind of thing would incentive that.

I'd keep the bounties directory, if only to see if any interesting bounty-related contributions come our way.

bool public claimed;
mapping(address => address) public researchers;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ pragma solidity ^0.4.24;
* See https://github.com/ethereum/solidity/issues/864
*/

library ECRecovery {
library ECDSA {

/**
* @dev Recover signer address from a message by using their signature
Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion contracts/mocks/AutoIncrementingImpl.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
pragma solidity ^0.4.24;

import "../AutoIncrementing.sol";
import "../utils/AutoIncrementing.sol";


contract AutoIncrementingImpl {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
pragma solidity ^0.4.24;


import "../ECRecovery.sol";
import "../cryptography/ECDSA.sol";


contract ECRecoveryMock {
using ECRecovery for bytes32;
contract ECDSAMock {
using ECDSA for bytes32;

function recover(bytes32 _hash, bytes _signature)
public
Expand Down
20 changes: 20 additions & 0 deletions contracts/mocks/InsecureInvariantTargetBounty.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
pragma solidity ^0.4.24;

// When this line is split, truffle parsing fails.
// See: https://github.com/ethereum/solidity/issues/4871
// solium-disable-next-line max-len
import {BreakInvariantBounty, Target} from "../../contracts/bounties/BreakInvariantBounty.sol";


contract InsecureInvariantTargetMock is Target {
function checkInvariant() public returns(bool) {
return false;
}
}


contract InsecureInvariantTargetBounty is BreakInvariantBounty {
function deployContract() internal returns (address) {
return new InsecureInvariantTargetMock();
}
}
17 changes: 0 additions & 17 deletions contracts/mocks/InsecureTargetBounty.sol

This file was deleted.

13 changes: 0 additions & 13 deletions contracts/mocks/LimitBalanceMock.sol

This file was deleted.

2 changes: 1 addition & 1 deletion contracts/mocks/MerkleProofWrapper.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
pragma solidity ^0.4.24;

import { MerkleProof } from "../MerkleProof.sol";
import { MerkleProof } from "../cryptography/MerkleProof.sol";


contract MerkleProofWrapper {
Expand Down
2 changes: 1 addition & 1 deletion contracts/mocks/ReentrancyMock.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
pragma solidity ^0.4.24;

import "../ReentrancyGuard.sol";
import "../utils/ReentrancyGuard.sol";
import "./ReentrancyAttack.sol";


Expand Down
20 changes: 20 additions & 0 deletions contracts/mocks/SecureInvariantTargetBounty.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
pragma solidity ^0.4.24;

// When this line is split, truffle parsing fails.
// See: https://github.com/ethereum/solidity/issues/4871
// solium-disable-next-line max-len
import {BreakInvariantBounty, Target} from "../../contracts/bounties/BreakInvariantBounty.sol";


contract SecureInvariantTargetMock is Target {
function checkInvariant() public returns(bool) {
return true;
}
}


contract SecureInvariantTargetBounty is BreakInvariantBounty {
function deployContract() internal returns (address) {
return new SecureInvariantTargetMock();
}
}
17 changes: 0 additions & 17 deletions contracts/mocks/SecureTargetBounty.sol

This file was deleted.

4 changes: 2 additions & 2 deletions contracts/token/ERC721/ERC721Basic.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pragma solidity ^0.4.24;
import "./IERC721Basic.sol";
import "./IERC721Receiver.sol";
import "../../math/SafeMath.sol";
import "../../AddressUtils.sol";
import "../../utils/Address.sol";
import "../../introspection/SupportsInterfaceWithLookup.sol";


Expand All @@ -14,7 +14,7 @@ import "../../introspection/SupportsInterfaceWithLookup.sol";
contract ERC721Basic is SupportsInterfaceWithLookup, IERC721Basic {

using SafeMath for uint256;
using AddressUtils for address;
using Address for address;

// Equals to `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`
// which can be also obtained as `IERC721Receiver(0).onERC721Received.selector`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pragma solidity ^0.4.24;
/**
* Utility library of inline functions on addresses
*/
library AddressUtils {
library Address {

/**
* Returns whether the target address is a contract
Expand Down
File renamed without changes.
File renamed without changes.
10 changes: 5 additions & 5 deletions test/Bounty.test.js β†’ test/BreakInvariantBounty.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ const { ethGetBalance, ethSendTransaction } = require('./helpers/web3');
const expectEvent = require('./helpers/expectEvent');
const { assertRevert } = require('./helpers/assertRevert');

const SecureTargetBounty = artifacts.require('SecureTargetBounty');
const InsecureTargetBounty = artifacts.require('InsecureTargetBounty');
const SecureInvariantTargetBounty = artifacts.require('SecureInvariantTargetBounty');
const InsecureInvariantTargetBounty = artifacts.require('InsecureInvariantTargetBounty');

require('chai')
.use(require('chai-bignumber')(web3.BigNumber))
Expand All @@ -17,10 +17,10 @@ const sendReward = async (from, to, value) => ethSendTransaction({

const reward = new web3.BigNumber(web3.toWei(1, 'ether'));

contract('Bounty', function ([_, owner, researcher, nonTarget]) {
contract('BreakInvariantBounty', function ([_, owner, researcher, nonTarget]) {
context('against secure contract', function () {
beforeEach(async function () {
this.bounty = await SecureTargetBounty.new({ from: owner });
this.bounty = await SecureInvariantTargetBounty.new({ from: owner });
});

it('can set reward', async function () {
Expand Down Expand Up @@ -53,7 +53,7 @@ contract('Bounty', function ([_, owner, researcher, nonTarget]) {

context('against broken contract', function () {
beforeEach(async function () {
this.bounty = await InsecureTargetBounty.new();
this.bounty = await InsecureInvariantTargetBounty.new();

const result = await this.bounty.createTarget({ from: researcher });
const event = expectEvent.inLogs(result.logs, 'TargetCreated');
Expand Down
59 changes: 0 additions & 59 deletions test/LimitBalance.test.js

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
const { signMessage, toEthSignedMessageHash } = require('../helpers/sign');
const { expectThrow } = require('../helpers/expectThrow');

const ECRecoveryMock = artifacts.require('ECRecoveryMock');
const ECDSAMock = artifacts.require('ECDSAMock');

require('chai')
.should();

const TEST_MESSAGE = web3.sha3('OpenZeppelin');
const WRONG_MESSAGE = web3.sha3('Nope');

contract('ECRecovery', function ([_, anyone]) {
contract('ECDSA', function ([_, anyone]) {
beforeEach(async function () {
this.mock = await ECRecoveryMock.new();
this.mock = await ECDSAMock.new();
});

it('recover v0', async function () {
Expand Down