-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix: Update noripple_check to exclude transactions field on error responses
#6303
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: develop
Are you sure you want to change the base?
Changes from 1 commit
3367bce
4807dae
6c3bc27
b896342
9b5ad16
52d09a6
ab1f99c
726c077
ff7ec76
9500529
c032ca4
239c706
2d44eaa
25f0321
7fd457a
42ba358
b162fcf
7d1aee8
f63b325
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 |
|---|---|---|
|
|
@@ -102,13 +102,16 @@ class NoRippleCheck_test : public beast::unit_test::suite | |
|
|
||
| { // passing an account private key will cause | ||
| // parsing as a seed to fail | ||
| // also, `transactions` is not included | ||
| Json::Value params; | ||
| 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."); | ||
| BEAST_EXPECT(!result.isMember(jss::transactions)); | ||
| } | ||
|
|
||
| { | ||
|
|
@@ -198,12 +201,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; | ||
|
Comment on lines
+212
to
+213
|
||
| if (!BEAST_EXPECT(txs.size() == (user ? 1 : 2))) | ||
| return; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,12 +22,12 @@ 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(); | ||
| } | ||
|
|
||
| // { | ||
|
|
@@ -42,32 +42,40 @@ Json::Value | |
| doNoRippleCheck(RPC::JsonContext& context) | ||
| { | ||
| auto const& params(context.params); | ||
| if (!params.isMember(jss::account)) | ||
| return RPC::missing_field_error("account"); | ||
|
|
||
| if (!params.isMember("role")) | ||
| return RPC::missing_field_error("role"); | ||
| // check account param | ||
| if (!params.isMember(jss::account)) | ||
| return RPC::missing_field_error(jss::account); | ||
|
|
||
| if (!params[jss::account].isString()) | ||
| return RPC::invalid_field_error(jss::account); | ||
|
|
||
| auto id = parseBase58<AccountID>(params[jss::account].asString()); | ||
| if (!id) | ||
| { | ||
| return rpcError(rpcACT_MALFORMED); | ||
| } | ||
|
Comment on lines
+54
to
+58
|
||
| auto const accountID{std::move(id.value())}; | ||
|
|
||
| // check role param | ||
| if (!params.isMember(jss::role)) | ||
| return RPC::missing_field_error(jss::role); | ||
|
|
||
| bool roleGateway = false; | ||
| { | ||
| std::string const role = params["role"].asString(); | ||
| if (role == "gateway") | ||
| std::string const role = params[jss::role].asString(); | ||
| if (role == jss::gateway) | ||
| roleGateway = true; | ||
| else if (role != "user") | ||
| return RPC::invalid_field_error("role"); | ||
| else if (role != jss::user) | ||
| return RPC::invalid_field_error(jss::role); | ||
|
Comment on lines
+62
to
+71
|
||
| } | ||
|
|
||
| // check limit param | ||
| unsigned int limit; | ||
| if (auto err = readLimitField(limit, RPC::Tuning::noRippleCheck, context)) | ||
| return *err; | ||
|
|
||
| bool transactions = false; | ||
| if (params.isMember(jss::transactions)) | ||
| transactions = params["transactions"].asBool(); | ||
|
|
||
| // check transactions param | ||
| // 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 | ||
|
|
@@ -77,31 +85,28 @@ doNoRippleCheck(RPC::JsonContext& context) | |
| return RPC::invalid_field_error(jss::transactions); | ||
| } | ||
|
|
||
| bool transactions = false; | ||
| if (params.isMember(jss::transactions)) | ||
| transactions = params[jss::transactions].asBool(); | ||
|
|
||
| // lookup ledger via params | ||
| std::shared_ptr<ReadView const> ledger; | ||
| auto result = RPC::lookupLedger(ledger, context); | ||
| if (!ledger) | ||
| return result; | ||
|
|
||
| Json::Value dummy; | ||
| Json::Value& jvTransactions = transactions ? (result[jss::transactions] = Json::arrayValue) : dummy; | ||
|
|
||
| auto id = parseBase58<AccountID>(params[jss::account].asString()); | ||
| if (!id) | ||
| { | ||
| RPC::inject_error(rpcACT_MALFORMED, result); | ||
| return result; | ||
| } | ||
| auto const accountID{std::move(id.value())}; | ||
| auto const sle = ledger->read(keylet::account(accountID)); | ||
| if (!sle) | ||
| return rpcError(rpcACT_NOT_FOUND); | ||
|
|
||
| std::uint32_t seq = sle->getFieldU32(sfSequence); | ||
|
|
||
| Json::Value& problems = (result["problems"] = Json::arrayValue); | ||
| Json::Value& problems = (result[jss::problems] = Json::arrayValue); | ||
|
|
||
| bool bDefaultRipple = sle->getFieldU32(sfFlags) & lsfDefaultRipple; | ||
|
|
||
| Json::Value jvTransactions = Json::arrayValue; | ||
|
|
||
| if (bDefaultRipple & !roleGateway) | ||
mvadari marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { | ||
|
Comment on lines
+112
to
113
|
||
| problems.append( | ||
|
|
@@ -115,8 +120,8 @@ doNoRippleCheck(RPC::JsonContext& context) | |
| if (transactions) | ||
| { | ||
| Json::Value& tx = jvTransactions.append(Json::objectValue); | ||
| tx["TransactionType"] = jss::AccountSet; | ||
| tx["SetFlag"] = 8; | ||
| tx[jss::TransactionType] = jss::AccountSet; | ||
| tx[jss::SetFlag] = 8; | ||
| fillTransaction(context, tx, accountID, seq, *ledger); | ||
| } | ||
| } | ||
|
|
@@ -153,9 +158,9 @@ doNoRippleCheck(RPC::JsonContext& context) | |
| limitAmount.setIssuer(peer); | ||
|
|
||
| Json::Value& tx = jvTransactions.append(Json::objectValue); | ||
| tx["TransactionType"] = jss::TrustSet; | ||
| tx["LimitAmount"] = limitAmount.getJson(JsonOptions::none); | ||
| tx["Flags"] = bNoRipple ? tfClearNoRipple : tfSetNoRipple; | ||
| tx[jss::TransactionType] = jss::TrustSet; | ||
| tx[jss::LimitAmount] = limitAmount.getJson(JsonOptions::none); | ||
| tx[jss::Flags] = bNoRipple ? tfClearNoRipple : tfSetNoRipple; | ||
| fillTransaction(context, tx, accountID, seq, *ledger); | ||
|
|
||
| return true; | ||
|
|
@@ -164,6 +169,7 @@ doNoRippleCheck(RPC::JsonContext& context) | |
| return false; | ||
| }); | ||
|
|
||
| result[jss::transactions] = std::move(jvTransactions); | ||
| return result; | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.