feat: allow debtor consent delegation in DebtKernel #184
Conversation
There was a problem hiding this comment.
IMO, if we're in the business of updating charta already, we should apply this delegation system more generally (i.e. not just for debtors, but also for creditors). I think you could separate out all of the logic in 349 - 362 into a reusable function which will make this much cleaner
| @@ -76,6 +76,8 @@ contract DebtKernel is Pausable { | |||
| mapping (bytes32 => bool) public issuanceCancelled; | |||
| mapping (bytes32 => bool) public debtOrderCancelled; | |||
|
|
|||
| mapping (address => address) public debtorConsentDelegation; | |||
There was a problem hiding this comment.
Feels odd to just do this for debtors and not for other actors in the protocol — in particular, I think there are many reasons this could be useful for creditors as well. Would pretty dramatically simplify the way creditor proxies work, for instance.
| if (!isValidSignature( | ||
| debtOrder.issuance.debtor, | ||
| delegatedDebtor, |
There was a problem hiding this comment.
This approach only delegates the ability for the given delegate to sign on the debtor's behalf, but not the ability to more broadly do anything on the debtor's behalf that would require the debtor's consent in some way. IMO this delegation should be more expansive — it should be possible, for instance, for the delegate to be a contract (which can't sign messages, but can submit them as a msg.sender).
I'd recommend putting the delegation logic outside of the if (msg.sender... check and including the delegatedDebtor in that logic in that.
There was a problem hiding this comment.
I was thinking about delegation of consent more broadly, and agree that it would make for a much more expressive DebtKernel.
My main worry is about unnecessarily expanding scope to beyond what we need today and potentially introducing bugs, vulnerabilities, etc.
U think we're moving towards a world where these contracts (core settlement contracts + dharma lever contracts) are more upgradeable, which is why I'd advocate with moving forward with the minimum set of work required to get this working for trustless relay.
If you guys feel that there is imminent feature work that will leverage consent delegation for creditors at the DebtKernel level, etc., I'm more than happy to make those code changes.
cc: @graemecode
No description provided.