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

Agreement: Remove ETH as an option for fees #602

Merged
merged 4 commits into from
Jul 27, 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
2 changes: 1 addition & 1 deletion contracts/apps/disputable/DisputableAragonApp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ contract DisputableAragonApp is IDisputable, AragonApp {
*/
function _newAgreementAction(uint256 _disputableActionId, bytes _context, address _submitter) internal returns (uint256) {
IAgreement agreement = _ensureAgreement();
return agreement.newAction.value(msg.value)(_disputableActionId, _context, _submitter);
return agreement.newAction(_disputableActionId, _context, _submitter);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion contracts/apps/disputable/IAgreement.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ contract IAgreement is IArbitrable, IACLOracle {

function deactivate(address _disputable) external;

function newAction(uint256 _disputableActionId, bytes _context, address _submitter) external payable returns (uint256);
function newAction(uint256 _disputableActionId, bytes _context, address _submitter) external returns (uint256);

function closeAction(uint256 _actionId) external;

Expand Down
2 changes: 1 addition & 1 deletion contracts/lib/arbitration/IAragonAppFeesCashier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ interface IAragonAppFeesCashier {
function setAppFees(bytes32[] _appIds, ERC20[] _tokens, uint256[] _amounts) external;
function unsetAppFee(bytes32 _appId) external;
function unsetAppFees(bytes32[] _appIds) external;
function payAppFees(bytes32 _appId, bytes _data) external payable;
function payAppFees(bytes32 _appId, bytes _data) external;
function getAppFee(bytes32 _appId) external view returns (ERC20, uint256);
}
122 changes: 120 additions & 2 deletions contracts/test/mocks/apps/disputable/AgreementMock.sol
Original file line number Diff line number Diff line change
@@ -1,13 +1,131 @@
pragma solidity 0.4.24;

import "../../../../apps/disputable/IAgreement.sol";

contract AgreementMock {
function newAction(uint256 /* _disputableActionId */, bytes /* _context */, address /* _submitter */) external payable returns (uint256) {

contract AgreementMock is IAgreement {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you will need to mock all the methods then

function sign() external {
// do nothing
}

function activate(
address,
ERC20,
uint256,
uint256,
uint64
)
external
{
// do nothing
}

function deactivate(address) external {
// do nothing
}

function newAction(uint256 /* _disputableActionId */, bytes /* _context */, address /* _submitter */) external returns (uint256) {
// do nothing
return 0;
}

function closeAction(uint256 /* _actionId */) external {
// do nothing
}

function challengeAction(uint256, uint256, bool, bytes) external {
// do nothing
}

function settleAction(uint256) external {
// do nothing
}

function disputeAction(uint256, bool) external {
// do nothing
}

function getSigner(address) external view returns (uint256, bool) {
// do nothing
}

function getCurrentSettingId() external view returns (uint256) {
// do nothing
}

function getSetting(uint256) external view
returns (
IArbitrator,
IAragonAppFeesCashier,
string,
bytes
)
{
// do nothing
}

function getDisputableInfo(address) external view returns (bool, uint256) {
// do nothing
}

function getCollateralRequirement(address, uint256) external view
returns (
ERC20,
uint256,
uint256,
uint64
)
{
// do nothing
}

function getAction(uint256) external view
returns (
address,
uint256,
uint256,
uint256,
address,
bool,
bytes,
uint256
)
{
// do nothing
}

function getChallenge(uint256) external view
returns (
uint256,
address,
uint64,
bytes,
uint256,
uint256,
ERC20,
ChallengeState,
bool,
bool,
uint256,
uint256
)
{
// do nothing
}

function submitEvidence(uint256 _disputeId, bytes _evidence, bool _finished) external {
// do nothing
}

function rule(uint256 _disputeId, uint256 _ruling) external {
// do nothing
}

function supportsInterface(bytes4 _interfaceId) external pure returns (bool) {
// do nothing
}

function canPerform(address, address, address, bytes32, uint256[]) external view returns (bool) {
// do nothing
}
}
2 changes: 1 addition & 1 deletion contracts/test/mocks/apps/disputable/DisputableAppMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ contract DisputableAppMock is DisputableAragonApp {
initialized();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we're missing one instance in the AgreementMock.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, if you can also clean up a few compiler warnings about this one it would be really helpful!

}

function newAction(uint256 _disputableActionId, bytes _context, address _submitter) external payable {
function newAction(uint256 _disputableActionId, bytes _context, address _submitter) external {
_newAgreementAction(_disputableActionId, _context, _submitter);
}

Expand Down
9 changes: 0 additions & 9 deletions test/contracts/apps/disputable/disputable_app.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,15 +145,6 @@ contract('DisputableApp', ([_, owner, agreement, anotherAgreement, someone]) =>
it('does not revert', async () => {
await disputable.newAction(0, '0x00', owner)
})

it('receives ETH when sent', async () => {
const previousBalance = await web3.eth.getBalance(agreement.address)

await disputable.newAction(0, '0x00', owner, { value: 1e18 })

const currentBalance = await web3.eth.getBalance(agreement.address)
assert.equal(currentBalance.sub(previousBalance).toString(), 1e18, 'agreement balance does not match')
})
})
})

Expand Down