diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index ed27312023..192aa7272e 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -41,6 +41,7 @@ Version 3.4.0 is not yet released. These changes are available in the 3.4.0 beta - `gateway_balances`: The `account` and `ident` fields now return an `invalidParams` error if the value is not a string, instead of an `internal` error. [#7655](https://github.com/XRPLF/rippled/pull/7655) - `account_lines`: The `peer` field now returns an error if the value is not a string. [#7728](https://github.com/XRPLF/rippled/pull/7728) - `ledger`: `delivered_amount` is now included in the metadata of successful `AccountDelete` transactions when transactions are expanded (`expand`, or admin-only `full`). Previously it was only added for `Payment` and `CheckCash`, which made `ledger` inconsistent with `tx` and `account_tx`. [#5706](https://github.com/XRPLF/rippled/pull/5706) +- `noripple_check`: The `transactions` field is no longer included in error responses; it is still returned (possibly as an empty array) whenever `transactions` is `true` and the request succeeds. A malformed `account` is now rejected before the ledger is looked up, so that error response no longer carries the `ledger_hash`, `ledger_index`, and `validated` fields ([#6303](https://github.com/XRPLF/rippled/pull/6303)). ## XRP Ledger server version 3.3.0 diff --git a/include/xrpl/protocol/jss.h b/include/xrpl/protocol/jss.h index 63e877ca31..b294a846da 100644 --- a/include/xrpl/protocol/jss.h +++ b/include/xrpl/protocol/jss.h @@ -278,6 +278,7 @@ JSS(frozen_balances); // out: GatewayBalances JSS(full); // in: LedgerClearer, handlers/Ledger JSS(full_reply); // out: PathFind JSS(fullbelow_size); // out: GetCounts +JSS(gateway); // in: noripple_check JSS(git); // out: server_info JSS(good); // out: RPCVersion JSS(hash); // out: NetworkOPs, InboundLedger, LedgerToJson, STTx; field @@ -481,6 +482,7 @@ JSS(ports); // out: NetworkOPs JSS(previous); // out: Reservations JSS(previous_ledger); // out: LedgerPropose JSS(price); // out: amm_info, AuctionSlot +JSS(problems); // out: noripple_check JSS(proof); // in: BookOffers JSS(propose_seq); // out: LedgerPropose JSS(proposers); // out: NetworkOPs, LedgerConsensus @@ -660,6 +662,7 @@ JSS(url); // in/out: Subscribe, Unsubscribe JSS(url_password); // in: Subscribe JSS(url_username); // in: Subscribe JSS(urlgravatar); // +JSS(user); // in: noripple_check JSS(username); // in: Subscribe JSS(validated); // out: NetworkOPs, RPCHelpers, AccountTx*, Tx JSS(validator_list_expires); // out: NetworkOps, ValidatorList diff --git a/src/test/rpc/NoRippleCheck_test.cpp b/src/test/rpc/NoRippleCheck_test.cpp index 8e719e6407..3a4ddcf5c4 100644 --- a/src/test/rpc/NoRippleCheck_test.cpp +++ b/src/test/rpc/NoRippleCheck_test.cpp @@ -126,9 +126,16 @@ class NoRippleCheck_test : public beast::unit_test::Suite params[jss::account] = toBase58(TokenType::NodePrivate, alice.sk()); params[jss::role] = "user"; params[jss::ledger] = "current"; + params[jss::transactions] = true; auto const result = env.rpc("json", "noripple_check", to_string(params))[jss::result]; BEAST_EXPECT(result[jss::error] == "actMalformed"); BEAST_EXPECT(result[jss::error_message] == "Account malformed."); + // The changelog promises malformed-account responses carry + // neither `transactions` nor any ledger metadata. + BEAST_EXPECT(!result.isMember(jss::transactions)); + BEAST_EXPECT(!result.isMember(jss::ledger_hash)); + BEAST_EXPECT(!result.isMember(jss::ledger_index)); + BEAST_EXPECT(!result.isMember(jss::validated)); } { @@ -194,6 +201,7 @@ class NoRippleCheck_test : public beast::unit_test::Suite if (!BEAST_EXPECT(pa.isArray())) return; + BEAST_EXPECT(!result.isMember(jss::transactions)); if (problems) { if (!BEAST_EXPECT(pa.size() == 2)) @@ -219,12 +227,12 @@ class NoRippleCheck_test : public beast::unit_test::Suite // time. params[jss::transactions] = true; result = env.rpc("json", "noripple_check", to_string(params))[jss::result]; - if (!BEAST_EXPECT(result[jss::transactions].isArray())) - return; auto const txs = result[jss::transactions]; if (problems) { + if (!BEAST_EXPECT(result[jss::transactions].isArray())) + return; if (!BEAST_EXPECT(txs.size() == (user ? 1 : 2))) return; diff --git a/src/xrpld/rpc/handlers/account/NoRippleCheck.cpp b/src/xrpld/rpc/handlers/account/NoRippleCheck.cpp index 4be6e6f1af..283ef7cb3f 100644 --- a/src/xrpld/rpc/handlers/account/NoRippleCheck.cpp +++ b/src/xrpld/rpc/handlers/account/NoRippleCheck.cpp @@ -21,6 +21,8 @@ #include #include +#include +#include namespace xrpl { @@ -32,12 +34,13 @@ fillTransaction( std::uint32_t& sequence, ReadView const& ledger) { - txArray["Sequence"] = json::UInt(sequence++); - txArray["Account"] = toBase58(accountID); + txArray[jss::Sequence] = json::UInt(sequence++); + txArray[jss::Account] = toBase58(accountID); auto& fees = ledger.fees(); // Convert the reference transaction cost in fee units to drops // scaled to represent the current fee load. - txArray["Fee"] = scaleFeeLoad(fees.base, context.app.getFeeTrack(), fees, false).jsonClipped(); + txArray[jss::Fee] = + scaleFeeLoad(fees.base, context.app.getFeeTrack(), fees, false).jsonClipped(); } // { @@ -53,24 +56,33 @@ doNoRippleCheck(rpc::JsonContext& context) { auto const& params(context.params); if (!params.isMember(jss::account)) - return rpc::missingFieldError("account"); - - if (!params.isMember("role")) - return rpc::missingFieldError("role"); + return rpc::missingFieldError(jss::account); if (!params[jss::account].isString()) return rpc::invalidFieldError(jss::account); + auto id = parseBase58(params[jss::account].asString()); + if (!id) + { + return rpcError(RpcActMalformed); + } + auto const accountID{id.value()}; + + if (!params.isMember(jss::role)) + return rpc::missingFieldError(jss::role); + bool roleGateway = false; { - std::string const role = params["role"].asString(); - if (role == "gateway") + if (!params[jss::role].isString()) + return rpc::expectedFieldError(jss::role, "string"); + std::string const role = params[jss::role].asString(); + if (role == jss::gateway) { roleGateway = true; } - else if (role != "user") + else if (role != jss::user) { - return rpc::invalidFieldError("role"); + return rpc::invalidFieldError(jss::role); } } @@ -78,61 +90,49 @@ doNoRippleCheck(rpc::JsonContext& context) if (auto err = readLimitField(limit, rpc::tuning::kNoRippleCheck, context)) return *err; - bool transactions = false; - if (params.isMember(jss::transactions)) - transactions = params["transactions"].asBool(); - - // The document[https://xrpl.org/noripple_check.html#noripple_check] states - // that transactions params is a boolean value, however, assigning any - // string value works. Do not allow this. This check is for api Version 2 - // onwards only + // API v1 silently accepts any string as `transactions`; v2+ enforces bool. if (context.apiVersion > 1u && params.isMember(jss::transactions) && !params[jss::transactions].isBool()) { return rpc::invalidFieldError(jss::transactions); } + bool transactions = false; + if (params.isMember(jss::transactions)) + transactions = params[jss::transactions].asBool(); + std::shared_ptr ledger; auto result = rpc::lookupLedger(ledger, context); if (!ledger) return result; - json::Value dummy; // NOLINT(misc-const-correctness) - json::Value& jvTransactions = - transactions ? (result[jss::transactions] = json::ValueType::Array) : dummy; - - auto id = parseBase58(params[jss::account].asString()); - if (!id) - { - rpc::injectError(RpcActMalformed, result); - return result; - } - auto const accountID{id.value()}; auto const sle = ledger->read(keylet::account(accountID)); if (!sle) return rpcError(RpcActNotFound); std::uint32_t seq = sle->getFieldU32(sfSequence); - json::Value& problems = (result["problems"] = json::ValueType::Array); + json::Value& problems = (result[jss::problems] = json::ValueType::Array); - bool const bDefaultRipple = sle->isFlag(lsfDefaultRipple); + bool const defaultRipple = sle->isFlag(lsfDefaultRipple); - if (bDefaultRipple && !roleGateway) + json::Value jvTransactions = json::ValueType::Array; + + if (defaultRipple && !roleGateway) { problems.append( "You appear to have set your default ripple flag even though you " "are not a gateway. This is not recommended unless you are " "experimenting"); } - else if (roleGateway && !bDefaultRipple) + else if (roleGateway && !defaultRipple) { problems.append("You should immediately set your default ripple flag"); if (transactions) { json::Value& tx = jvTransactions.append(json::ValueType::Object); - tx["TransactionType"] = jss::AccountSet; - tx["SetFlag"] = 8; + tx[jss::TransactionType] = jss::AccountSet; + tx[jss::SetFlag] = 8; fillTransaction(context, tx, accountID, seq, *ledger); } } @@ -140,18 +140,18 @@ doNoRippleCheck(rpc::JsonContext& context) forEachItemAfter(*ledger, accountID, uint256(), 0, limit, [&](SLE::const_ref ownedItem) { if (ownedItem->getType() == ltRIPPLE_STATE) { - bool const bLow = accountID == ownedItem->getFieldAmount(sfLowLimit).getIssuer(); + bool const low = accountID == ownedItem->getFieldAmount(sfLowLimit).getIssuer(); - bool const bNoRipple = ownedItem->isFlag(bLow ? lsfLowNoRipple : lsfHighNoRipple); + bool const noRipple = ownedItem->isFlag(low ? lsfLowNoRipple : lsfHighNoRipple); std::string problem; bool needFix = false; - if (bNoRipple && roleGateway) + if (noRipple && roleGateway) { problem = "You should clear the no ripple flag on your "; needFix = true; } - else if (!roleGateway && !bNoRipple) + else if (!roleGateway && !noRipple) { problem = "You should probably set the no ripple flag on your "; needFix = true; @@ -159,22 +159,25 @@ doNoRippleCheck(rpc::JsonContext& context) if (needFix) { AccountID const peer = - ownedItem->getFieldAmount(bLow ? sfHighLimit : sfLowLimit).getIssuer(); + ownedItem->getFieldAmount(low ? sfHighLimit : sfLowLimit).getIssuer(); STAmount const peerLimit = - ownedItem->getFieldAmount(bLow ? sfHighLimit : sfLowLimit); + ownedItem->getFieldAmount(low ? sfHighLimit : sfLowLimit); problem += to_string(peerLimit.get().currency); problem += " line to "; problem += to_string(peerLimit.getIssuer()); problems.append(problem); - STAmount limitAmount(ownedItem->getFieldAmount(bLow ? sfLowLimit : sfHighLimit)); + STAmount limitAmount(ownedItem->getFieldAmount(low ? sfLowLimit : sfHighLimit)); limitAmount.get().account = peer; - json::Value& tx = jvTransactions.append(json::ValueType::Object); - tx["TransactionType"] = jss::TrustSet; - tx["LimitAmount"] = limitAmount.getJson(JsonOptions::Values::None); - tx["Flags"] = bNoRipple ? tfClearNoRipple : tfSetNoRipple; - fillTransaction(context, tx, accountID, seq, *ledger); + if (transactions) + { + json::Value& tx = jvTransactions.append(json::ValueType::Object); + tx[jss::TransactionType] = jss::TrustSet; + tx[jss::LimitAmount] = limitAmount.getJson(JsonOptions::Values::None); + tx[jss::Flags] = noRipple ? tfClearNoRipple : tfSetNoRipple; + fillTransaction(context, tx, accountID, seq, *ledger); + } return true; } @@ -182,6 +185,8 @@ doNoRippleCheck(rpc::JsonContext& context) return false; }); + if (transactions) + result[jss::transactions] = std::move(jvTransactions); return result; }