-
Notifications
You must be signed in to change notification settings - Fork 50
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
feat: allow debtor consent delegation in DebtKernel #184
base: master
Are you sure you want to change the base?
Changes from 1 commit
bf24aa9
41123c7
5b43424
1288f8c
43153d3
f7d6c56
7f28f51
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,6 +76,8 @@ contract DebtKernel is Pausable { | |
mapping (bytes32 => bool) public issuanceCancelled; | ||
mapping (bytes32 => bool) public debtOrderCancelled; | ||
|
||
mapping (address => address) public debtorConsentDelegation; | ||
|
||
event LogDebtOrderFilled( | ||
bytes32 indexed _agreementId, | ||
uint _principal, | ||
|
@@ -292,6 +294,14 @@ contract DebtKernel is Pausable { | |
LogDebtOrderCancelled(debtOrder.debtOrderHash, msg.sender); | ||
} | ||
|
||
function delegateDebtorConsent( | ||
address delegate | ||
) | ||
public | ||
{ | ||
debtorConsentDelegation[msg.sender] = delegate; | ||
} | ||
|
||
//////////////////////// | ||
// INTERNAL FUNCTIONS // | ||
//////////////////////// | ||
|
@@ -337,8 +347,14 @@ contract DebtKernel is Pausable { | |
{ | ||
// Invariant: debtor's signature must be valid, unless debtor is submitting order | ||
chrismin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (msg.sender != debtOrder.issuance.debtor) { | ||
address delegatedDebtor = debtOrder.issuance.debtor; | ||
|
||
if (debtorConsentDelegation[debtOrder.issuance.debtor] != address(0)) { | ||
delegatedDebtor = debtorConsentDelegation[debtOrder.issuance.debtor]; | ||
} | ||
|
||
if (!isValidSignature( | ||
debtOrder.issuance.debtor, | ||
delegatedDebtor, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This approach only delegates the ability for the given I'd recommend putting the delegation logic outside of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
debtOrder.debtOrderHash, | ||
signaturesV[0], | ||
signaturesR[0], | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.