refactor: Split LoanInvariant into LoanBrokerInvariant and LoanInvariant #6674
+250
−229
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| #pragma once | ||
|
|
||
| #include <xrpl/basics/base_uint.h> | ||
| #include <xrpl/beast/utility/Journal.h> | ||
| #include <xrpl/ledger/ReadView.h> | ||
| #include <xrpl/protocol/STTx.h> | ||
| #include <xrpl/protocol/TER.h> | ||
|
|
||
| #include <map> | ||
| #include <vector> | ||
|
|
||
| namespace xrpl { | ||
|
|
||
| /** | ||
| * @brief Invariants: Loan brokers are internally consistent | ||
| * | ||
| * 1. If `LoanBroker.OwnerCount = 0` the `DirectoryNode` will have at most one | ||
| * node (the root), which will only hold entries for `RippleState` or | ||
| * `MPToken` objects. | ||
| * | ||
| */ | ||
| class ValidLoanBroker | ||
| { | ||
| // Not all of these elements will necessarily be populated. Remaining items | ||
| // will be looked up as needed. | ||
| struct BrokerInfo | ||
| { | ||
| SLE::const_pointer brokerBefore = nullptr; | ||
| // After is used for most of the checks, except | ||
| // those that check changed values. | ||
| SLE::const_pointer brokerAfter = nullptr; | ||
| }; | ||
| // Collect all the LoanBrokers found directly or indirectly through | ||
| // pseudo-accounts. Key is the brokerID / index. It will be used to find the | ||
| // LoanBroker object if brokerBefore and brokerAfter are nullptr | ||
| std::map<uint256, BrokerInfo> brokers_; | ||
| // Collect all the modified trust lines. Their high and low accounts will be | ||
| // loaded to look for LoanBroker pseudo-accounts. | ||
| std::vector<SLE::const_pointer> lines_; | ||
| // Collect all the modified MPTokens. Their accounts will be loaded to look | ||
| // for LoanBroker pseudo-accounts. | ||
| std::vector<SLE::const_pointer> mpts_; | ||
|
|
||
| static bool | ||
| goodZeroDirectory(ReadView const& view, SLE::const_ref dir, beast::Journal const& j); | ||
|
|
||
| public: | ||
| void | ||
| visitEntry(bool, std::shared_ptr<SLE const> const&, std::shared_ptr<SLE const> const&); | ||
|
|
||
| bool | ||
| finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&); | ||
| }; | ||
|
|
||
| } // namespace xrpl |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,194 @@ | ||
| #include <xrpl/tx/invariants/LoanBrokerInvariant.h> | ||
| // | ||
| #include <xrpl/basics/Log.h> | ||
| #include <xrpl/beast/utility/instrumentation.h> | ||
| #include <xrpl/ledger/View.h> | ||
| #include <xrpl/ledger/helpers/RippleStateHelpers.h> | ||
| #include <xrpl/protocol/Indexes.h> | ||
| #include <xrpl/protocol/LedgerFormats.h> | ||
| #include <xrpl/protocol/STNumber.h> | ||
| #include <xrpl/protocol/TxFormats.h> | ||
|
|
||
| namespace xrpl { | ||
|
|
||
| void | ||
| ValidLoanBroker::visitEntry( | ||
| bool isDelete, | ||
| std::shared_ptr<SLE const> const& before, | ||
| std::shared_ptr<SLE const> const& after) | ||
| { | ||
| if (after) | ||
| { | ||
| if (after->getType() == ltLOAN_BROKER) | ||
| { | ||
| auto& broker = brokers_[after->key()]; | ||
| broker.brokerBefore = before; | ||
| broker.brokerAfter = after; | ||
| } | ||
| else if (after->getType() == ltACCOUNT_ROOT && after->isFieldPresent(sfLoanBrokerID)) | ||
| { | ||
| auto const& loanBrokerID = after->at(sfLoanBrokerID); | ||
| // create an entry if one doesn't already exist | ||
| brokers_.emplace(loanBrokerID, BrokerInfo{}); | ||
| } | ||
| else if (after->getType() == ltRIPPLE_STATE) | ||
| { | ||
| lines_.emplace_back(after); | ||
| } | ||
| else if (after->getType() == ltMPTOKEN) | ||
| { | ||
| mpts_.emplace_back(after); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| bool | ||
| ValidLoanBroker::goodZeroDirectory( | ||
| ReadView const& view, | ||
| SLE::const_ref dir, | ||
| beast::Journal const& j) | ||
| { | ||
| auto const next = dir->at(~sfIndexNext); | ||
| auto const prev = dir->at(~sfIndexPrevious); | ||
| if ((prev && (*prev != 0u)) || (next && (*next != 0u))) | ||
| { | ||
| JLOG(j.fatal()) << "Invariant failed: Loan Broker with zero " | ||
| "OwnerCount has multiple directory pages"; | ||
| return false; | ||
| } | ||
| auto indexes = dir->getFieldV256(sfIndexes); | ||
| if (indexes.size() > 1) | ||
| { | ||
| JLOG(j.fatal()) << "Invariant failed: Loan Broker with zero " | ||
| "OwnerCount has multiple indexes in the Directory root"; | ||
| return false; | ||
| } | ||
| if (indexes.size() == 1) | ||
| { | ||
| auto const index = indexes.value().front(); | ||
| auto const sle = view.read(keylet::unchecked(index)); | ||
| if (!sle) | ||
| { | ||
| JLOG(j.fatal()) << "Invariant failed: Loan Broker directory corrupt"; | ||
| return false; | ||
| } | ||
| if (sle->getType() != ltRIPPLE_STATE && sle->getType() != ltMPTOKEN) | ||
| { | ||
| JLOG(j.fatal()) << "Invariant failed: Loan Broker with zero " | ||
| "OwnerCount has an unexpected entry in the directory"; | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| bool | ||
| ValidLoanBroker::finalize( | ||
| STTx const& tx, | ||
| TER const, | ||
| XRPAmount const, | ||
| ReadView const& view, | ||
| beast::Journal const& j) | ||
| { | ||
| // Loan Brokers will not exist on ledger if the Lending Protocol amendment | ||
| // is not enabled, so there's no need to check it. | ||
|
|
||
| for (auto const& line : lines_) | ||
| { | ||
| for (auto const& field : {&sfLowLimit, &sfHighLimit}) | ||
| { | ||
| auto const account = view.read(keylet::account(line->at(*field).getIssuer())); | ||
Tapanito marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // This Invariant doesn't know about the rules for Trust Lines, so | ||
| // if the account is missing, don't treat it as an error. This | ||
| // loop is only concerned with finding Broker pseudo-accounts | ||
| if (account && account->isFieldPresent(sfLoanBrokerID)) | ||
| { | ||
| auto const& loanBrokerID = account->at(sfLoanBrokerID); | ||
| // create an entry if one doesn't already exist | ||
| brokers_.emplace(loanBrokerID, BrokerInfo{}); | ||
| } | ||
| } | ||
| } | ||
| for (auto const& mpt : mpts_) | ||
| { | ||
| auto const account = view.read(keylet::account(mpt->at(sfAccount))); | ||
Tapanito marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // This Invariant doesn't know about the rules for MPTokens, so | ||
| // if the account is missing, don't treat is as an error. This | ||
| // loop is only concerned with finding Broker pseudo-accounts | ||
| if (account && account->isFieldPresent(sfLoanBrokerID)) | ||
| { | ||
| auto const& loanBrokerID = account->at(sfLoanBrokerID); | ||
| // create an entry if one doesn't already exist | ||
| brokers_.emplace(loanBrokerID, BrokerInfo{}); | ||
| } | ||
| } | ||
|
|
||
| for (auto const& [brokerID, broker] : brokers_) | ||
| { | ||
| auto const& after = | ||
| broker.brokerAfter ? broker.brokerAfter : view.read(keylet::loanbroker(brokerID)); | ||
|
|
||
| if (!after) | ||
| { | ||
| JLOG(j.fatal()) << "Invariant failed: Loan Broker missing"; | ||
| return false; | ||
| } | ||
|
|
||
| auto const& before = broker.brokerBefore; | ||
|
|
||
| // https://github.com/Tapanito/XRPL-Standards/blob/xls-66-lending-protocol/XLS-0066d-lending-protocol/README.md#3123-invariants | ||
| // If `LoanBroker.OwnerCount = 0` the `DirectoryNode` will have at most | ||
| // one node (the root), which will only hold entries for `RippleState` | ||
| // or `MPToken` objects. | ||
| if (after->at(sfOwnerCount) == 0) | ||
| { | ||
| auto const dir = view.read(keylet::ownerDir(after->at(sfAccount))); | ||
| if (dir) | ||
| { | ||
| if (!goodZeroDirectory(view, dir, j)) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
| if (before && before->at(sfLoanSequence) > after->at(sfLoanSequence)) | ||
| { | ||
| JLOG(j.fatal()) << "Invariant failed: Loan Broker sequence number " | ||
| "decreased"; | ||
| return false; | ||
| } | ||
| if (after->at(sfDebtTotal) < 0) | ||
| { | ||
| JLOG(j.fatal()) << "Invariant failed: Loan Broker debt total is negative"; | ||
|
Collaborator
There was a problem hiding this comment. @Tapanito Although a structural change only, as you told the AI bot, it would be a good idea to add:
here and below, if these places are not reached via unit tests / cannot be reached ever. It's fine to do this in a follow-up PR, and probably should be part of a larger, comprehensive effort to do this properly across all invariants.
Collaborator
Author
There was a problem hiding this comment. Thanks @bthomee, I will do this as part of Lending Protocol invariants. They are a little bit scant at the moment. |
||
| return false; | ||
| } | ||
| if (after->at(sfCoverAvailable) < 0) | ||
| { | ||
| JLOG(j.fatal()) << "Invariant failed: Loan Broker cover available is negative"; | ||
| return false; | ||
| } | ||
| auto const vault = view.read(keylet::vault(after->at(sfVaultID))); | ||
| if (!vault) | ||
| { | ||
| JLOG(j.fatal()) << "Invariant failed: Loan Broker vault ID is invalid"; | ||
| return false; | ||
| } | ||
| auto const& vaultAsset = vault->at(sfAsset); | ||
| if (after->at(sfCoverAvailable) < accountHolds( | ||
| view, | ||
| after->at(sfAccount), | ||
| vaultAsset, | ||
| FreezeHandling::fhIGNORE_FREEZE, | ||
| AuthHandling::ahIGNORE_AUTH, | ||
| j)) | ||
| { | ||
| JLOG(j.fatal()) << "Invariant failed: Loan Broker cover available " | ||
| "is less than pseudo-account asset balance"; | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| } // namespace xrpl | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
keylet::uncheckedweakens invariant safety — directory entries can hold heterogeneous types, but add a comment explaining whyuncheckedis required here: