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

Update expenditure functionality #1031

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

Update expenditure functionality #1031

wants to merge 1 commit into from

Conversation

area
Copy link
Member

@area area commented Apr 14, 2022

After a call with Raul and Arren yesterday, discussion how expenditures wished to be used, three issues raised themselves. The first commit adds tests demonstrating two:

  • When voting is conducted on an expenditure, the claim delay is reset once all disputes are resolved, whereas clearly the original claim delay should be left (if appropriate). This should also, presumably, apply in a slightly different way to motions about the claim delay (which I have not included a test for)
  • When arbitration mode is used to change the payout for a motion (which is currently done via the general-purpose setExpenditureState), the payoutsWeCannotMake value for the associated funding pot is not updated.

I think it's clear both of the above should be resolved. The third issue is less clear on how to resolve. It currently takes a lot of motions to make a complicated expenditure, and there's some indication that the ability to make a motion to e.g. set a lot of slots simultaneously, and possibly fund them, would be attractive - but the issue there is the only reasonable approach to such an implementation would want to use makeArbitraryTransactions, which forbids self-targeting for security reasons.

I think the best solution to this third issue is an extension built on top of expenditures akin to OneTxPayment with whatever batched functionality the front-end requires.

@area
Copy link
Member Author

area commented Apr 14, 2022

Is the expenditureLocked modifier defined, but not used at all?

@jakzilla
Copy link
Contributor

jakzilla commented Apr 19, 2022

1/ I don't quite understand the issue.

There are 2 claim delays on an expenditure. First, the colony's default claim delay which applies to all expenditures, allowing time for dispute. Second is the optional claim delay which may be additionally configured on each slot of an expenditure. Which are we referring to? For the purposes of this I am assuming we're referring to the default colony wide claim delay.For avoidance of doubt, by voting, do you mean any conclusion of a motion about changing some parameter of the expenditure?

Then finally do you mean that if the claim delay was 24h and the dispute started after 12h, then upon resolution of the dispute currently the clock would be reset to 24h, wheras you believe it should recommence with the time that would have remained had there not been a dispute?

If the claim delay being disputed is the default claim delay, would any change arising from the dispute only effect the exenditure in question or all other claim delays? What if the default value was disputed separately to any ongoing expenditures?

2/ I don't understand this issue. Do I need to?

3/ We were cognisant of the UX issue of many motions when creating an expenditure, and I believe @kronosapiens wrote an extension contract which would allow a user to supply a stake in order to "get" an expenditure. That would then permit them to unilaterally set any parameter of the expenditure up until it is "locked", after which changes may be made via motions. Are you highlighting this problem as of concern during the creation or amendment of an expenditure. If the former, I believe it's already dealt with, if the latter then I'd suggest we not try to prematurely optimise and see how much of an issue it is in practice.

@arrenv
Copy link
Member

arrenv commented Apr 19, 2022

@area @jakzilla this PR seems to give a lot of context - #958

@area
Copy link
Member Author

area commented Apr 19, 2022

There are 2 claim delays on an expenditure. First, the colony's default claim delay which applies to all expenditures, allowing time for dispute. Second is the optional claim delay which may be additionally configured on each slot of an expenditure. Which are we referring to?

Everything described here applies to both.

For avoidance of doubt, by voting, do you mean any conclusion of a motion about changing some parameter of the expenditure?

Yes

Then finally do you mean that if the claim delay was 24h and the dispute started after 12h, then upon resolution of the dispute currently the clock would be reset to 24h, wheras you believe it should recommence with the time that would have remained had there not been a dispute?

No, I mean that if the claim delay was 24 hours and the dispute started after 12h, then currently the clock is reset to 0h. This is what I brought up at the end of the PR that Arren has linked.

If the claim delay being disputed is the default claim delay, would any change arising from the dispute only effect the exenditure in question or all other claim delays? What if the default value was disputed separately to any ongoing expenditures?

The global claim delay for an expenditure is set to the default claim delay on creation. If the default claim delay is changed or disputed (by any mechanism), that doesn't affect any expenditures already created. If the global claim delay for an expenditure is changed or disputed, that only affects that expenditure.

2 I don't understand this issue. Do I need to?

I mean, depends on how much you're interested. The upshot of it is if the arbitration permission is used to change the payout of an expenditure, it can render the expenditure bricked (until the correct actions are taken with the arbitration and funding permissions) if that change should change the 'funded' state of the expenditure.

3/ We were cognisant of the UX issue of many motions when creating an expenditure, and I believe @kronosapiens wrote an extension contract which would allow a user to supply a stake in order to "get" an expenditure. That would then permit them to unilaterally set any parameter of the expenditure up until it is "locked", after which changes may be made via motions. Are you highlighting this problem as of concern during the creation or amendment of an expenditure. If the former, I believe it's already dealt with, if the latter then I'd suggest we not try to prematurely optimise and see how much of an issue it is in practice.

The existence of this extension is news to me, so I guess I'll wait for Daniel to weigh in.

@jakzilla
Copy link
Contributor

Ah, okay so then, for 1 I would agree that it going to 0 if any motion is created about it is not desirable (though this is presumably true only if the motion is fully staked?). We need then to decide whether we want the claim delay to reset to whatever its previous value was (e.g. 24h in the example I gave) +/- any difference in the claim delay if the change targeted it? Or alternatively, whether it should change to whatever amount of time was left before the dispute took place, +/- any change to the claim delay?

I guess the former would be technically easier, and safer as the latter would make it possible for an undesirable change to sneak through in the closing seconds of a legitimate expenditure (even if the motion process should prevent this from happening in the first place).

@kronosapiens
Copy link
Member

kronosapiens commented Apr 19, 2022

Hey all,

  • Regarding managing the claim delay, I noticed this issue and addressed it in the PR to add Token & Hybrid voting (see here)
  • Regarding managing payoutsWeCannotMake -- good catch. We can probably add some logic to the setExpenditureState function which would check this under the right conditions (not such an arbitrary state change function after all!)
  • Regarding managing complex expenditures, a few notes. We did discuss allowing non-admin users to "check out" an expenditure via a stake (essentially borrowing the admin permission), but that (simple) extension has not yet been implemented. It would take some of the burden off of the motions process by allowing a user to set up the expenditure in a lightweight way, going to a motion only for e.g. the funding. Another option would be to update the motions contract to support multi-transaction motions (which I believe we've discussed but have yet to implement), along the lines of makeArbitraryTransactions. That would introduce some issues around gas costs but nothing we couldn't handle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants