Add Reimbursement app #198
@ -6,6 +6,7 @@ import "@aragon/os/contracts/kernel/IKernel.sol";
|
||||
|
|
||||
contract Reimbursement is AragonApp {
|
||||
bytes32 public constant ADD_REIMBURSEMENT_ROLE = keccak256("ADD_REIMBURSEMENT_ROLE");
|
||||
bytes32 public constant VETO_REIMBURSEMENT_ROLE = keccak256("VETO_REIMBURSEMENT_ROLE");
|
||||
// bytes32 public constant MANAGE_APPS_ROLE = keccak256("MANAGE_APPS_ROLE");
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
|
||||
struct ReimbursementData {
|
||||
uint32 recipientId;
|
||||
@ -27,13 +28,14 @@ contract Reimbursement is AragonApp {
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
|
||||
event ReimbursementVetoed(uint32 id, address vetoedByAccount);
|
||||
|
||||
// TODO: remove _appIds when those are removed from the kreditskit
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
// using the appids to find other apps is wrong according to aragon
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
function initialize(bytes32[5] _appIds) public onlyInit {
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
function initialize() public onlyInit {
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
blocksToWait = 40320; // 7 days; 15 seconds block time
|
||||
initialized();
|
||||
}
|
||||
|
||||
// function setApps() public isInitialized auth(MANAGE_APPS_ROLE) {
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
// }
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
function totalAmount(bool confirmedOnly) public view returns (uint256 amount) {
|
||||
for (uint32 i = 1; i <= reimbursementsCount; i++) {
|
||||
ReimbursementData memory r = reimbursements[i];
|
||||
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
@ -50,7 +50,7 @@ contract KreditsKit is KitBase {
|
||||
proposal.initialize(appIds);
|
||||
|
||||
Reimbursement reimbursement = Reimbursement(_installApp(dao, appIds[uint8(Apps.Reimbursement)]));
|
||||
reimbursement.initialize(appIds);
|
||||
reimbursement.initialize();
|
||||
acl.createPermission(root, reimbursement, reimbursement.ADD_REIMBURSEMENT_ROLE(), this);
|
||||
acl.createPermission(root, reimbursement, reimbursement.VETO_REIMBURSEMENT_ROLE(), this);
|
||||
|
||||
|
||||
(Although I'm not sure if something like
submittedByisn't nicer for a name there.)(Although I'm not sure if something like
submittedByisn't nicer for a name there.)Maybe we should just call this
get, analog toaddbelow?Maybe we should just call this
get, analog toaddbelow?I think until we actually have the financial code done, we should not add this function or the
claimedproperty. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).I think until we actually have the financial code done, we should not add this function or the
claimedproperty. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).yeah, thought about that, too. but we had
getContributionin the contribution contract.do you think that's the best pattern? 👍
yeah, thought about that, too. but we had
getContributionin the contribution contract.do you think that's the best pattern? 👍
Yeah, I think it should be consistent, and
contribution.addis nicer for clients thancontribution.addContribution. May as well imply the scope for all functions then. (We actually throw a deprecation warning foraddContributionstill. :))Yeah, I think it should be consistent, and
contribution.addis nicer for clients thancontribution.addContribution. May as well imply the scope for all functions then. (We actually throw a deprecation warning foraddContributionstill. :))