diff --git a/src/test/app/Credentials_test.cpp b/src/test/app/Credentials_test.cpp index 18b2f761b5..1d1b71034f 100644 --- a/src/test/app/Credentials_test.cpp +++ b/src/test/app/Credentials_test.cpp @@ -34,15 +34,6 @@ namespace ripple { namespace test { -static inline bool -checkVL( - std::shared_ptr const& sle, - SField const& field, - std::string const& expected) -{ - return strHex(expected) == strHex(sle->getFieldVL(field)); -} - struct Credentials_test : public beast::unit_test::suite { void diff --git a/src/test/app/DID_test.cpp b/src/test/app/DID_test.cpp index 34aa54f234..9232d9059f 100644 --- a/src/test/app/DID_test.cpp +++ b/src/test/app/DID_test.cpp @@ -27,14 +27,6 @@ namespace ripple { namespace test { -bool -checkVL(Slice const& result, std::string expected) -{ - Serializer s; - s.addRaw(result); - return s.getString() == expected; -} - struct DID_test : public beast::unit_test::suite { void diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 5acf3f1f84..2f55696761 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -96,7 +96,8 @@ class LoanBroker_test : public beast::unit_test::suite // Evan will attempt to be naughty Account evan{"evan"}; Vault vault{env}; - env.fund(XRP(1000), issuer, alice, evan); + env.fund(XRP(1000), issuer, noripple(alice, evan)); + env.close(); // Create assets PrettyAsset const xrpAsset{xrpIssue(), 1'000'000}; @@ -137,6 +138,7 @@ class LoanBroker_test : public beast::unit_test::suite env(vault.deposit( {.depositor = alice, .id = keylet.key, .amount = asset(50)})); + env.close(); } // Create and update Loan Brokers @@ -144,47 +146,145 @@ class LoanBroker_test : public beast::unit_test::suite { using namespace loanBroker; - // Try some failure cases - env(set(evan, vault.vaultID), ter(tecNO_PERMISSION)); - // flags are checked first - env(set(evan, vault.vaultID, ~tfUniversal), ter(temINVALID_FLAG)); - // field length validation - // sfData: good length, bad account - env(set(evan, vault.vaultID), - data(strHex(std::string(maxDataPayloadLength, '0'))), - ter(tecNO_PERMISSION)); - // sfData: too long - env(set(evan, vault.vaultID), - data(strHex(std::string(maxDataPayloadLength + 1, '0'))), - ter(temINVALID)); - // sfManagementFeeRate: good value, bad account - env(set(evan, vault.vaultID), - managementFeeRate(maxFeeRate), - ter(tecNO_PERMISSION)); - // sfManagementFeeRate: too big - env(set(evan, vault.vaultID), - managementFeeRate(maxFeeRate + 1), - ter(temINVALID)); - // sfCoverRateMinimum: good value, bad account - env(set(evan, vault.vaultID), - coverRateMinimum(maxCoverRate), - ter(tecNO_PERMISSION)); - // sfCoverRateMinimum: too big - env(set(evan, vault.vaultID), - coverRateMinimum(maxCoverRate + 1), - ter(temINVALID)); - // sfCoverRateLiquidation: good value, bad account - env(set(evan, vault.vaultID), - coverRateLiquidation(maxCoverRate), - ter(tecNO_PERMISSION)); - // sfCoverRateLiquidation: too big - env(set(evan, vault.vaultID), - coverRateLiquidation(maxCoverRate + 1), - ter(temINVALID)); + { + auto badKeylet = keylet::vault(alice.id(), env.seq(alice)); + // Try some failure cases + // not the vault owner + env(set(evan, vault.vaultID), ter(tecNO_PERMISSION)); + // not a vault + env(set(alice, badKeylet.key), ter(tecNO_ENTRY)); + // flags are checked first + env(set(evan, vault.vaultID, ~tfUniversal), + ter(temINVALID_FLAG)); + // field length validation + // sfData: good length, bad account + env(set(evan, vault.vaultID), + data(strHex(std::string(maxDataPayloadLength, '0'))), + ter(tecNO_PERMISSION)); + // sfData: too long + env(set(evan, vault.vaultID), + data(strHex(std::string(maxDataPayloadLength + 1, '0'))), + ter(temINVALID)); + // sfManagementFeeRate: good value, bad account + env(set(evan, vault.vaultID), + managementFeeRate(maxFeeRate), + ter(tecNO_PERMISSION)); + // sfManagementFeeRate: too big + env(set(evan, vault.vaultID), + managementFeeRate(maxFeeRate + 1), + ter(temINVALID)); + // sfCoverRateMinimum: good value, bad account + env(set(evan, vault.vaultID), + coverRateMinimum(maxCoverRate), + ter(tecNO_PERMISSION)); + // sfCoverRateMinimum: too big + env(set(evan, vault.vaultID), + coverRateMinimum(maxCoverRate + 1), + ter(temINVALID)); + // sfCoverRateLiquidation: good value, bad account + env(set(evan, vault.vaultID), + coverRateLiquidation(maxCoverRate), + ter(tecNO_PERMISSION)); + // sfCoverRateLiquidation: too big + env(set(evan, vault.vaultID), + coverRateLiquidation(maxCoverRate + 1), + ter(temINVALID)); - auto keylet = keylet::loanbroker(alice.id(), env.seq(alice)); - env(set(alice, vault.vaultID)); - BEAST_EXPECT(env.le(keylet)); + auto keylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vault.vaultID)); + env.close(); + auto broker = env.le(keylet); + if (BEAST_EXPECT(broker)) + { + // Check the fields + BEAST_EXPECT(broker->at(sfVaultID) == vault.vaultID); + BEAST_EXPECT(broker->at(sfAccount) != alice.id()); + BEAST_EXPECT(broker->at(sfOwner) == alice.id()); + BEAST_EXPECT(!broker->isFieldPresent(sfManagementFeeRate)); + BEAST_EXPECT(!broker->isFieldPresent(sfCoverRateMinimum)); + BEAST_EXPECT( + !broker->isFieldPresent(sfCoverRateLiquidation)); + BEAST_EXPECT(broker->at(sfFlags) == 0); + BEAST_EXPECT(broker->at(sfSequence) == env.seq(alice) - 1); + BEAST_EXPECT(!broker->isFieldPresent(sfData)); + BEAST_EXPECT(broker->at(sfOwnerCount) == 0); + BEAST_EXPECT(broker->at(sfDebtTotal) == 0); + BEAST_EXPECT(broker->at(sfDebtMaximum) == 0); + BEAST_EXPECT(broker->at(sfCoverAvailable) == 0); + BEAST_EXPECT(broker->at(sfCoverRateMinimum) == 0); + BEAST_EXPECT(broker->at(sfCoverRateLiquidation) == 0); + + // Load the pseudo-account + + // Update the fields + auto nextKeylet = + keylet::loanbroker(alice.id(), env.seq(alice)); + + // no-op + env(set(alice, vault.vaultID), loanBrokerID(keylet.key)); + + // fields that can't be changed + // LoanBrokerID + env(set(alice, vault.vaultID), + loanBrokerID(nextKeylet.key), + ter(tecNO_ENTRY)); + // VaultID + env(set(alice, nextKeylet.key), + loanBrokerID(keylet.key), + ter(tecNO_PERMISSION)); + // Owner + env(set(evan, vault.vaultID), + loanBrokerID(keylet.key), + ter(tecNO_PERMISSION)); + // ManagementFeeRate + env(set(alice, vault.vaultID), + loanBrokerID(keylet.key), + managementFeeRate(maxFeeRate), + ter(temINVALID)); + // CoverRateMinimum + env(set(alice, vault.vaultID), + loanBrokerID(keylet.key), + coverRateMinimum(maxFeeRate), + ter(temINVALID)); + // CoverRateLiquidation + env(set(alice, vault.vaultID), + loanBrokerID(keylet.key), + coverRateLiquidation(maxFeeRate), + ter(temINVALID)); + + // fields that can be changed + std::string const testData("Test Data 1234"); + // Bad data must be hex encoded + try + { + env(set(alice, vault.vaultID), + loanBrokerID(keylet.key), + data(testData), + ter(temINVALID)); + fail(); + } + catch (std::exception const& e) + { + BEAST_EXPECT( + e.what() == + std::string("invalidParamsField 'tx_json.Data' has " + "invalid data.")); + } + // Bad debt maximum + // Data & Debt maximum + env(set(alice, vault.vaultID), + loanBrokerID(keylet.key), + data(strHex(testData)), + debtMaximum(Number(175, -1))); + env.close(); + // Check the updated fields + broker = env.le(keylet); + BEAST_EXPECT(checkVL(broker->at(sfData), testData)); + BEAST_EXPECT( + broker->at(sfDebtMaximum) == + Number(175, -1)); + } + } } } diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index fa85227d92..1492463b9d 100644 --- a/src/test/jtx/TestHelpers.h +++ b/src/test/jtx/TestHelpers.h @@ -289,6 +289,25 @@ checkArraySize(Json::Value const& val, unsigned int size); std::uint32_t ownerCount(test::jtx::Env const& env, test::jtx::Account const& account); +inline +[[nodiscard]] bool +checkVL(Slice const& result, std::string expected) +{ + Serializer s; + s.addRaw(result); + return s.getString() == expected; +} + +inline +[[nodiscard]] bool +checkVL( + std::shared_ptr const& sle, + SField const& field, + std::string const& expected) +{ + return strHex(expected) == strHex(sle->getFieldVL(field)); +} + /* Path finding */ /******************************************************************************/ void diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 3908e16b84..a0c5a4a1db 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -1783,7 +1783,11 @@ ValidPseudoAccounts::visitEntry( lsfDisableMaster | lsfDefaultRipple | lsfDepositAuth)) { errors_.emplace_back( - "Invariant failed: pseudo-account flags are not set"); + "pseudo-account flags are not set"); + } + if (after->isFieldPresent(sfRegularKey)) + { + errors_.emplace_back("pseudo-account has a regular key"); } } } diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index bce695c990..a5dbca31a4 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -539,6 +539,18 @@ Transactor::apply() NotTEC Transactor::checkSign(PreclaimContext const& ctx) { + { + auto const id = ctx.tx.getAccountID(sfAccount); + + auto const sle = ctx.view.read(keylet::account(id)); + + if (ctx.view.rules().enabled(featureLendingProtocol) && + isPseudoAccount(sle)) + // Pseudo-accounts can't sign transactions. This check is gated on + // the Lending Protocol amendment because that's the project it was + // added under, and it doesn't justify another amendment + return tefBAD_AUTH; + } if (ctx.flags & tapDRY_RUN) { // This code must be different for `simulate`