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.

Sign up for GitHub

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

Jump to bottom

Disputable apps: add missing pieces for transaction fees module #586

Merged
merged 6 commits into from
Jun 30, 2020
Merged

Disputable apps: add missing pieces for transaction fees module #586

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
73ddf84
Disputable apps: add missing pieces for transaction fees module
Jun 22, 2020
f40902e
arbitration: Add IAragonCourtArbitrator
Jun 26, 2020
1e97ddd
arbitration: Remove IAragonCourtArbitrator
Jun 26, 2020
fe3148f
5.0.0-beta.1
Jun 29, 2020
3c202d8
disputable apps: fixes after rebase on next
Jun 29, 2020
91fd176
disputable apps: Add ITransactionFeesOracle to Agreement settings
Jun 30, 2020
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
4 changes: 3 additions & 1 deletion contracts/apps/disputable/IDisputable.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import "../../lib/standards/ERC165.sol";

contract IDisputable is ERC165 {
bytes4 internal constant ERC165_INTERFACE_ID = bytes4(0x01ffc9a7);
bytes4 internal constant DISPUTABLE_INTERFACE_ID = bytes4(0xef113021);
bytes4 internal constant DISPUTABLE_INTERFACE_ID = bytes4(0xce1f6de9);

event AgreementSet(IAgreement indexed agreement);

Expand Down Expand Up @@ -39,4 +39,6 @@ contract IDisputable is ERC165 {
function supportsInterface(bytes4 _interfaceId) external pure returns (bool) {
return _interfaceId == DISPUTABLE_INTERFACE_ID || _interfaceId == ERC165_INTERFACE_ID;
}

function appId() public view returns (bytes32);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can leverage the new breaking version we are working on to make these external

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What’s the benefit?

Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid having the compiler warning:

Warning: Functions in interfaces should be declared external.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where do you see that warning? I can’t find it. Despite the name, IDisputable is not actually an interface, so the warning is not showing up for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure to be honest, however this is a minor detail, not a blocker at all

Copy link
Contributor

@sohkai sohkai Jun 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Somewhat related discussion, but maybe we should create a new interface, e.g. IAragonApp to enforce the kernel() and appId() functions?

That way IDisputable can declare that it's always an IAragonApp (and we can adjust the EIP-165 values to be separated between IDisputable and IAragonApp).

We could also move out supportsInterface() out of the base app—it's kind of weird that it implements it as an app may choose to support any number of interfaces (e.g. the token receiver ones from 721 or 1155).

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the idea of IAragonApp, I'll create an issue for that :)

}
12 changes: 12 additions & 0 deletions contracts/lib/arbitration/ITransactionFeesOracle.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
pragma solidity ^0.4.24;

import "../token/ERC20.sol";


interface ITransactionFeesOracle {
function setFee(bytes32 appId, ERC20 token, uint256 amount) external;
function setFees(bytes32[] _appIds, ERC20[] _tokens, uint256[] _amounts) external;
function unsetFee(bytes32 _appId) external;
function unsetFees(bytes32[] _appIds) external;
function getFee(bytes32 appId) external view returns (ERC20, uint256, address);
}
bingen marked this conversation as resolved.
Show resolved Hide resolved
14 changes: 9 additions & 5 deletions contracts/test/mocks/apps/disputable/DisputableAppMock.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,15 @@ contract DisputableAppMock is DisputableAragonApp {
function interfaceID() external pure returns (bytes4) {
IDisputable iDisputable;
return iDisputable.setAgreement.selector ^
iDisputable.onDisputableActionChallenged.selector ^
iDisputable.onDisputableActionAllowed.selector ^
iDisputable.onDisputableActionRejected.selector ^
iDisputable.onDisputableActionVoided.selector ^
iDisputable.getAgreement.selector;
iDisputable.onDisputableActionChallenged.selector ^
iDisputable.onDisputableActionAllowed.selector ^
iDisputable.onDisputableActionRejected.selector ^
iDisputable.onDisputableActionVoided.selector ^
iDisputable.getAgreement.selector ^
iDisputable.getDisputableAction.selector ^
bingen marked this conversation as resolved.
Show resolved Hide resolved
iDisputable.canChallenge.selector ^
iDisputable.canClose.selector ^
iDisputable.appId.selector;
}

function erc165interfaceID() external pure returns (bytes4) {
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@aragon/os",
"version": "5.0.0-beta.0",
"version": "5.0.0-beta.1",
"description": "Core contracts for Aragon",
"scripts": {
"compile": "truffle compile",
Expand Down
10 changes: 6 additions & 4 deletions test/contracts/apps/disputable/disputable_app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ contract('DisputableApp', ([_, owner, agreement, anotherAgreement, someone]) =>
let disputable, disputableBase, dao, acl

const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'
const DISPUTABLE_INTERFACE = '0xce1f6de9'
const ERC165_INTERFACE = '0x01ffc9a7'

before('deploy DAO', async () => {
const kernelBase = await Kernel.new(true)
Expand Down Expand Up @@ -41,16 +43,16 @@ contract('DisputableApp', ([_, owner, agreement, anotherAgreement, someone]) =>

describe('supportsInterface', () => {
it('supports ERC165', async () => {
assert.isTrue(await disputable.supportsInterface('0x01ffc9a7'), 'does not support ERC165')
assert.isTrue(await disputable.supportsInterface(ERC165_INTERFACE), 'does not support ERC165')

assert.equal(await disputable.ERC165_INTERFACE(), '0x01ffc9a7', 'ERC165 interface ID does not match')
assert.equal(await disputable.ERC165_INTERFACE(), ERC165_INTERFACE, 'ERC165 interface ID does not match')
assert.equal(await disputable.erc165interfaceID(), await disputable.ERC165_INTERFACE(), 'ERC165 interface ID does not match')
})

it('supports IDisputable', async () => {
assert.isTrue(await disputable.supportsInterface('0xef113021'), 'does not support IDisputable')
assert.isTrue(await disputable.supportsInterface(DISPUTABLE_INTERFACE), 'does not support IDisputable')

assert.equal(await disputable.interfaceID(), '0xef113021')
assert.equal(await disputable.interfaceID(), DISPUTABLE_INTERFACE)
assert.equal(await disputable.DISPUTABLE_INTERFACE(), await disputable.interfaceID(), 'IDisputable interface ID does not match')
})

Expand Down