From ea8e77ffec065cf1a8d1cd4517f9cebdab27cc17 Mon Sep 17 00:00:00 2001 From: Oleksandr <115580134+oleks-rip@users.noreply.github.com> Date: Mon, 7 Oct 2024 12:44:45 -0400 Subject: [PATCH] RXI-1295 review fixes --- include/xrpl/protocol/Credentials.h | 36 -------------------- src/test/app/DepositAuth_test.cpp | 6 ++-- src/test/jtx/impl/credentials.cpp | 1 - src/xrpld/app/tx/detail/Credentials.cpp | 38 ++++++++++------------ src/xrpld/app/tx/detail/DepositPreauth.cpp | 11 +++++-- 5 files changed, 30 insertions(+), 62 deletions(-) delete mode 100644 include/xrpl/protocol/Credentials.h diff --git a/include/xrpl/protocol/Credentials.h b/include/xrpl/protocol/Credentials.h deleted file mode 100644 index 29f8797a259..00000000000 --- a/include/xrpl/protocol/Credentials.h +++ /dev/null @@ -1,36 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2012, 2013 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - -#pragma once - -#include -#include -#include -#include - -namespace ripple { - -inline void -serializeCredential(Serializer& msg, uint256 const& key) -{ - msg.add32(HashPrefix::credential); - msg.addBitString(key); -} - -} // namespace ripple diff --git a/src/test/app/DepositAuth_test.cpp b/src/test/app/DepositAuth_test.cpp index 08725108be6..6351b4135af 100644 --- a/src/test/app/DepositAuth_test.cpp +++ b/src/test/app/DepositAuth_test.cpp @@ -1013,7 +1013,7 @@ struct DepositPreauth_test : public beast::unit_test::suite } { - // invalid account + // invalid issuer auto jv = deposit::authCredentials(bob, {}); auto& arr(jv[sfAuthorizeCredentials.jsonName]); Json::Value jcred = Json::objectValue; @@ -1053,10 +1053,10 @@ struct DepositPreauth_test : public beast::unit_test::suite } { - // Can create with non-existing issuer + // Can't create with non-existing issuer Account const rick{"rick"}; auto jv = deposit::authCredentials(bob, {{rick, credType}}); - env(jv); + env(jv, ter(tecNO_ISSUER)); env.close(); } diff --git a/src/test/jtx/impl/credentials.cpp b/src/test/jtx/impl/credentials.cpp index 45907207ae1..7d10d04e18a 100644 --- a/src/test/jtx/impl/credentials.cpp +++ b/src/test/jtx/impl/credentials.cpp @@ -18,7 +18,6 @@ //============================================================================== #include -#include #include #include diff --git a/src/xrpld/app/tx/detail/Credentials.cpp b/src/xrpld/app/tx/detail/Credentials.cpp index 50db35b8e14..6b2f3565b04 100644 --- a/src/xrpld/app/tx/detail/Credentials.cpp +++ b/src/xrpld/app/tx/detail/Credentials.cpp @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include @@ -55,8 +54,7 @@ checkExpired( std::shared_ptr const& sle, NetClock::time_point const& closed) { - std::uint32_t const exp = - sle->isFieldPresent(sfExpiration) ? sle->getFieldU32(sfExpiration) : 0; + std::uint32_t const exp = (*sle)[~sfExpiration].value_or(0); std::uint32_t const now = closed.time_since_epoch().count(); return static_cast(exp) && (now > exp); } @@ -129,8 +127,8 @@ CredentialCreate::preflight(PreflightContext const& ctx) return temMALFORMED; } - auto const Uri = tx[~sfURI]; - if (Uri && (Uri->empty() || (Uri->size() > maxCredentialURILength))) + auto const uri = tx[~sfURI]; + if (uri && (uri->empty() || (uri->size() > maxCredentialURILength))) { JLOG(j.trace()) << "Malformed transaction: invalid size of URI."; return temMALFORMED; @@ -174,13 +172,6 @@ CredentialCreate::doApply() { auto const sleIssuer = view().peek(keylet::account(account_)); - { - STAmount const reserve{view().fees().accountReserve( - sleIssuer->getFieldU32(sfOwnerCount) + 1)}; - if (mPriorBalance < reserve) - return tecINSUFFICIENT_RESERVE; - } - auto const subject = ctx_.tx[sfSubject]; auto const issuer = account_; auto const credType(ctx_.tx[sfCredentialType]); @@ -203,6 +194,13 @@ CredentialCreate::doApply() sleCred->setFieldU32(sfExpiration, ctx_.tx.getFieldU32(sfExpiration)); } + { + STAmount const reserve{view().fees().accountReserve( + sleIssuer->getFieldU32(sfOwnerCount) + 1)}; + if (mPriorBalance < reserve) + return tecINSUFFICIENT_RESERVE; + } + sleCred->setAccountID(sfSubject, subject); sleCred->setAccountID(sfIssuer, issuer); sleCred->setFieldVL(sfCredentialType, credType); @@ -265,7 +263,7 @@ CredentialDelete::preflight(PreflightContext const& ctx) { // Neither field is present, the transaction is malformed. JLOG(ctx.j.trace()) << "Malformed transaction: " - "Invalid Subject and Issuer fields combination."; + "No Subject or Issuer fields."; return temMALFORMED; } @@ -435,12 +433,12 @@ CredentialAccept::doApply() AccountID const subject{account_}; AccountID const issuer{ctx_.tx[sfIssuer]}; - auto const sleSubj = view().peek(keylet::account(subject)); - auto const sleIss = view().peek(keylet::account(issuer)); + auto const sleSubject = view().peek(keylet::account(subject)); + auto const sleIssuer = view().peek(keylet::account(issuer)); { STAmount const reserve{view().fees().accountReserve( - sleSubj->getFieldU32(sfOwnerCount) + 1)}; + sleSubject->getFieldU32(sfOwnerCount) + 1)}; if (mPriorBalance < reserve) return tecINSUFFICIENT_RESERVE; } @@ -460,11 +458,11 @@ CredentialAccept::doApply() sleCred->setFieldU32(sfFlags, lsfAccepted); view().update(sleCred); - adjustOwnerCount(view(), sleIss, -1, j_); - view().update(sleIss); + adjustOwnerCount(view(), sleIssuer, -1, j_); + view().update(sleIssuer); - adjustOwnerCount(view(), sleSubj, 1, j_); - view().update(sleSubj); + adjustOwnerCount(view(), sleSubject, 1, j_); + view().update(sleSubject); return tesSUCCESS; } diff --git a/src/xrpld/app/tx/detail/DepositPreauth.cpp b/src/xrpld/app/tx/detail/DepositPreauth.cpp index 0b0fbccd14f..1e768578cf2 100644 --- a/src/xrpld/app/tx/detail/DepositPreauth.cpp +++ b/src/xrpld/app/tx/detail/DepositPreauth.cpp @@ -129,8 +129,8 @@ DepositPreauth::preflight(PreflightContext const& ctx) for (auto const& o : arr) { - AccountID const target(o[sfIssuer]); - if (!target) + AccountID const issuer(o[sfIssuer]); + if (!issuer) { JLOG(j.trace()) << "Malformed transaction: " @@ -179,6 +179,13 @@ DepositPreauth::preclaim(PreclaimContext const& ctx) else if (ctx.tx.isFieldPresent(sfAuthorizeCredentials)) { STArray const& authCred(ctx.tx.getFieldArray(sfAuthorizeCredentials)); + for (auto const& o : authCred) + { + AccountID const issuer(o[sfIssuer]); + if (!ctx.view.exists(keylet::account(issuer))) + return tecNO_ISSUER; + } + // Verify that the Preauth entry they asked to add is not already // in the ledger. if (ctx.view.exists(keylet::depositPreauth(account, authCred)))