Conversation
| import "../../lib/token/ERC20.sol"; | ||
|
|
||
|
|
||
| // Note |
There was a problem hiding this comment.
This is my primary note.
I've started thinking about the IAgreement as a "disputable action registry" (IDisputableActionRegistry), and backtracking on whether we need everything in this interface.
From a Disputable's standpoint, I think the main things its "action registry" should care about are:
- Registering new actions, which may be disputable
- Challenging an action
- Closing old actions
- Getting a little bit of information about a particular action or challenge
With that in mind, I started thinking that specific implementation-specific information about the "action registry" would be reduced and queried separately from the action disputable's information (e.g. when using subgraphs). For example, I noticed you only used the Agreement to find the vote's ID when disputes were created in the DisputableVoting subgraph.
There was a problem hiding this comment.
We decided not to force the IArbitrable and IACL inheritance at this level. The rest of the changes will be included in the aragonOS v6 roadmap
|
Thx for the audit @sohkai! I addressed your comments commit per commit, if it looks good we can release a new version and update the agreement and disputable voting apps |
sohkai
left a comment
There was a problem hiding this comment.
Last few things, I'm still wondering if we should eliminate more from IAgreement 🔪
|
@sohkai I like the idea! I addressed your comments, let me know your thoughts. About the app fees cashier inline doc, I had actually brought the content from the court repo, so I think we are fine :) |
Includes a number of small changes made during the audit, easiest to review commit by commit.
The only contract change:
IArbitrable: aligns its ERC165 implementation with the other contracts