-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Frozen IOUs should still be transferable to/from the issuer #6113
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
base: ximinez/lending-XLS-66-ongoing
Are you sure you want to change the base?
Changes from all commits
5fe00e3
933447c
a913a79
efae404
6d7e851
4e4e626
cb744e7
1b180ef
ee33367
18508d5
8523f42
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 |
|---|---|---|
|
|
@@ -1436,6 +1436,63 @@ class LoanBroker_test : public beast::unit_test::suite | |
| }); | ||
| } | ||
|
|
||
| void | ||
| testIssuerCoverDepositDuringGlobalFreeze() | ||
| { | ||
| testcase("Issuer can deposit to broker during global freeze"); | ||
| using namespace jtx; | ||
| using namespace loanBroker; | ||
|
|
||
| Account const issuer{"issuer"}; | ||
| Account const holder{"holder"}; | ||
| Env env(*this); | ||
| Vault vault{env}; | ||
|
|
||
| env.fund(XRP(100'000), issuer, holder); | ||
| env.close(); | ||
|
|
||
| PrettyAsset const asset = issuer["IOU"]; | ||
| env.trust(asset(1'000'000), holder); | ||
| env.close(); | ||
| env(pay(issuer, holder, asset(100'000))); | ||
| env.close(); | ||
|
|
||
| auto [tx, vaultKeylet] = | ||
| vault.create({.owner = issuer, .asset = asset}); | ||
| env(tx); | ||
| env.close(); | ||
|
|
||
| env(vault.deposit( | ||
| {.depositor = holder, .id = vaultKeylet.key, .amount = asset(50)})); | ||
| env.close(); | ||
|
|
||
| auto const brokerKeylet = | ||
| keylet::loanbroker(issuer.id(), env.seq(issuer)); | ||
| env(set(issuer, vaultKeylet.key)); | ||
| env.close(); | ||
|
|
||
| auto broker = env.le(brokerKeylet); | ||
| if (!BEAST_EXPECT(broker)) | ||
| return; | ||
|
|
||
| env(fset(issuer, asfGlobalFreeze)); | ||
| env.close(); | ||
|
|
||
| // Issuer CAN deposit to their own broker during global freeze | ||
| // This is the issuer exemption - issuer can always send (issue) their | ||
| // own tokens Per spec: "Counterparties of the frozen issuer can still | ||
| // send and receive payments directly to and from the issuing address." | ||
| env(coverDeposit(issuer, brokerKeylet.key, asset(10))); | ||
| env.close(); | ||
|
|
||
| // Verify the deposit succeeded | ||
| broker = env.le(brokerKeylet); | ||
| if (BEAST_EXPECT(broker)) | ||
| { | ||
| BEAST_EXPECT(broker->at(sfCoverAvailable) == asset(10).number()); | ||
| } | ||
|
Comment on lines
+1481
to
+1493
Collaborator
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. Could you also add tests to check that the broker can withdraw the cover, too? |
||
| } | ||
|
|
||
| void | ||
| testLoanBrokerSetDebtMaximum() | ||
| { | ||
|
|
@@ -1523,9 +1580,10 @@ class LoanBroker_test : public beast::unit_test::suite | |
| testInvalidLoanBrokerDelete(); | ||
| testInvalidLoanBrokerSet(); | ||
| testRequireAuth(); | ||
| testIssuerCoverDepositDuringGlobalFreeze(); | ||
|
|
||
| // TODO: Write clawback failure tests with an issuer / MPT that doesn't | ||
| // have the right flags set. | ||
| // TODO: Write clawback failure tests with an issuer / MPT that | ||
| // doesn't have the right flags set. | ||
| } | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,13 +80,23 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) | |
| return ter; | ||
|
|
||
| // Cannot withdraw from a Vault an Asset frozen for the destination account | ||
| if (auto const ret = checkFrozen(ctx.view, dstAcct, vaultAsset)) | ||
| return ret; | ||
| if (!vaultAsset.holds<Issue>() || | ||
| (dstAcct != vaultAsset.getIssuer() && | ||
| account != vaultAsset.getIssuer())) | ||
| { | ||
| if (auto const ret = checkFrozen(ctx.view, dstAcct, vaultAsset)) | ||
| return ret; | ||
| } | ||
|
|
||
| // Cannot return shares to the vault, if the underlying asset was frozen for | ||
| // the submitter | ||
| if (auto const ret = checkFrozen(ctx.view, account, vaultShare)) | ||
| return ret; | ||
| if (!vaultAsset.holds<Issue>() || | ||
| (dstAcct != vaultAsset.getIssuer() && | ||
| account != vaultAsset.getIssuer())) | ||
| { | ||
| if (auto const ret = checkFrozen(ctx.view, account, vaultShare)) | ||
| return ret; | ||
| } | ||
|
|
||
| return tesSUCCESS; | ||
| } | ||
|
|
@@ -115,6 +125,7 @@ VaultWithdraw::doApply() | |
|
|
||
| auto const amount = ctx_.tx[sfAmount]; | ||
| Asset const vaultAsset = vault->at(sfAsset); | ||
| auto const dstAcct = ctx_.tx[~sfDestination].value_or(account_); | ||
| MPTIssue const share{mptIssuanceID}; | ||
| STAmount sharesRedeemed = {share}; | ||
| STAmount assetsWithdrawn; | ||
|
|
@@ -165,11 +176,21 @@ VaultWithdraw::doApply() | |
| return tecPATH_DRY; | ||
| } | ||
|
|
||
| // When withdrawing IOU to the issuer, ignore freeze since spec allows | ||
| // returning frozen IOU assets to their issuer. MPTs don't have this | ||
| // exemption - MPT locks function like "deep freeze" with no issuer | ||
| // exception. | ||
|
Collaborator
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. @vlntb, do I understand correctly that, if the depositor is the issuer of an MPT, and the asset is locked, the issue will still be able to withdraw from the vault? |
||
| FreezeHandling const freezeHandling = (vaultAsset.holds<Issue>() && | ||
| (dstAcct == vaultAsset.getIssuer() || | ||
| account_ == vaultAsset.getIssuer())) | ||
| ? FreezeHandling::fhIGNORE_FREEZE | ||
| : FreezeHandling::fhZERO_IF_FROZEN; | ||
|
|
||
| if (accountHolds( | ||
| view(), | ||
| account_, | ||
| share, | ||
| FreezeHandling::fhZERO_IF_FROZEN, | ||
| freezeHandling, | ||
| AuthHandling::ahIGNORE_AUTH, | ||
| j_) < sharesRedeemed) | ||
| { | ||
|
|
@@ -237,8 +258,6 @@ VaultWithdraw::doApply() | |
| // else quietly ignore, account balance is not zero | ||
| } | ||
|
|
||
| auto const dstAcct = ctx_.tx[~sfDestination].value_or(account_); | ||
|
|
||
| return doWithdraw( | ||
| view(), | ||
| ctx_.tx, | ||
|
|
||
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.
Should also verify that the issuer can still send payments.