ARCHIVE/clio
1
0
Fork 0
mirror of https://github.com/XRPLF/clio.git synced 2025-11-04 11:55:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

Ledger_entry return invalid parameter error for v1 (#873)

Browse Source 
Fixes #875
This commit is contained in:
cyan317
2023-09-28 09:14:01 +01:00
committed by GitHub
parent e36545058d
commit 963685dd31
8 changed files with 153 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/rpc/common/APIVersion.h
View File

@@ -31,7 +31,7 @@ namespace rpc {
/**
* @brief Default API version to use if no version is specified by clients
*/
static constexpr uint32_t API_VERSION_DEFAULT = 2u;
static constexpr uint32_t API_VERSION_DEFAULT = 1u;
/**
* @brief Minimum API version supported by this build

4
src/rpc/common/impl/ForwardingProxy.h
View File

@@ -56,10 +56,6 @@ public:
if (ctx.method == "subscribe" || ctx.method == "unsubscribe")
return false;
// TODO: if needed, make configurable with json config option
if (ctx.apiVersion == 1)
return true;
if (handlerProvider_->isClioOnly(ctx.method))
return false;

21
src/rpc/handlers/AccountInfo.cpp
View File

@@ -80,10 +80,17 @@ AccountInfoHandler::process(AccountInfoHandler::Input input, Context const& ctx)
}
return Output(
lgrInfo.seq, ripple::strHex(lgrInfo.hash), sle, isDisallowIncomingEnabled, isClawbackEnabled, signerList);
lgrInfo.seq,
ripple::strHex(lgrInfo.hash),
sle,
isDisallowIncomingEnabled,
isClawbackEnabled,
ctx.apiVersion,
signerList);
}
return Output(lgrInfo.seq, ripple::strHex(lgrInfo.hash), sle, isDisallowIncomingEnabled, isClawbackEnabled);
return Output(
lgrInfo.seq, ripple::strHex(lgrInfo.hash), sle, isDisallowIncomingEnabled, isClawbackEnabled, ctx.apiVersion);
}
void
@@ -138,12 +145,10 @@ tag_invoke(boost::json::value_from_tag, boost::json::value& jv, AccountInfoHandl
std::cend(output.signerLists.value()),
std::back_inserter(signers),
[](auto const& signerList) { return toJson(signerList); });
// version 2 puts the signer_lists out of the account_data
jv.as_object()[JS(signer_lists)] = signers;
// this is a temporary fix to support the clients that still expect v1 api(the signer_lists under
// account_data)
// TODO: we need to deprecate the duplicate later
jv.as_object()[JS(account_data)].as_object()[JS(signer_lists)] = std::move(signers);
if (output.apiVersion == 1)
jv.as_object()[JS(account_data)].as_object()[JS(signer_lists)] = std::move(signers);
else
jv.as_object()[JS(signer_lists)] = signers;
}
}

3
src/rpc/handlers/AccountInfo.h
View File

@@ -44,6 +44,7 @@ public:
ripple::STLedgerEntry accountData;
bool isDisallowIncomingEnabled = false;
bool isClawbackEnabled = false;
uint32_t apiVersion;
std::optional<std::vector<ripple::STLedgerEntry>> signerLists;
// validated should be sent via framework
bool validated = true;
@@ -54,12 +55,14 @@ public:
ripple::STLedgerEntry sle,
bool isDisallowIncomingEnabled,
bool isClawbackEnabled,
uint32_t version,
std::optional<std::vector<ripple::STLedgerEntry>> signerLists = std::nullopt)
: ledgerIndex(ledgerId)
, ledgerHash(std::move(ledgerHash))
, accountData(std::move(sle))
, isDisallowIncomingEnabled(isDisallowIncomingEnabled)
, isClawbackEnabled(isClawbackEnabled)
, apiVersion(version)
, signerLists(std::move(signerLists))
{
}

2
src/rpc/handlers/LedgerEntry.cpp
View File

@@ -82,6 +82,8 @@ LedgerEntryHandler::process(LedgerEntryHandler::Input input, Context const& ctx)
else
{
// Must specify 1 of the following fields to indicate what type
if (ctx.apiVersion == 1)
return Error{Status{RippledError::rpcINVALID_PARAMS}};
return Error{Status{ClioError::rpcUNKNOWN_OPTION}};
}

8
unittests/rpc/ForwardingProxyTests.cpp
View File

@@ -290,19 +290,23 @@ TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsFalseIfLedgerAccountsIsFalse)
});
}
TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsTrueIfAPIVersionIsV1)
TEST_F(RPCForwardingProxyTest, ShouldNotForwardReturnsTrueIfAPIVersionIsV1)
{
auto const apiVersion = 1u;
auto const method = "api_version_check";
auto const params = json::parse("{}");
auto const rawHandlerProviderPtr = static_cast<MockHandlerProvider*>(handlerProvider.get());
ON_CALL(*rawHandlerProviderPtr, isClioOnly(_)).WillByDefault(Return(false));
EXPECT_CALL(*rawHandlerProviderPtr, isClioOnly(method)).Times(1);
runSpawn([&](auto yield) {
auto const range = mockBackendPtr->fetchLedgerRange();
auto const ctx =
web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP);
auto const res = proxy.shouldForward(ctx);
ASSERT_TRUE(res);
ASSERT_FALSE(res);
});
}

135
unittests/rpc/handlers/AccountInfoTests.cpp
View File

@@ -285,7 +285,7 @@ TEST_F(RPCAccountInfoHandlerTest, SignerListsInvalid)
});
}
TEST_F(RPCAccountInfoHandlerTest, SignerListsTrue)
TEST_F(RPCAccountInfoHandlerTest, SignerListsTrueV2)
{
auto const expectedOutput = fmt::format(
R"({{
@@ -300,37 +300,7 @@ TEST_F(RPCAccountInfoHandlerTest, SignerListsTrue)
"PreviousTxnLgrSeq": 2,
"Sequence": 2,
"TransferRate": 0,
"index": "13F1A95D7AAB7108D5CE7EEAF504B2894B8C674E6D68499076441C4837282BF8",
"signer_lists":
[
{{
"Flags": 0,
"LedgerEntryType": "SignerList",
"OwnerNode": "0",
"PreviousTxnID": "0000000000000000000000000000000000000000000000000000000000000000",
"PreviousTxnLgrSeq": 0,
"SignerEntries":
[
{{
"SignerEntry":
{{
"Account": "{}",
"SignerWeight": 1
}}
}},
{{
"SignerEntry":
{{
"Account": "{}",
"SignerWeight": 1
}}
}}
],
"SignerListID": 0,
"SignerQuorum": 2,
"index": "A9C28A28B85CD533217F5C0A0C7767666B093FA58A0F2D80026FCC4CD932DDC7"
}}
]
"index": "13F1A95D7AAB7108D5CE7EEAF504B2894B8C674E6D68499076441C4837282BF8"
}},
"signer_lists":
[
@@ -382,6 +352,105 @@ TEST_F(RPCAccountInfoHandlerTest, SignerListsTrue)
INDEX1,
ACCOUNT1,
ACCOUNT2,
LEDGERHASH);
auto const rawBackendPtr = static_cast<MockBackend*>(mockBackendPtr.get());
mockBackendPtr->updateRange(10); // min
mockBackendPtr->updateRange(30); // max
auto const ledgerinfo = CreateLedgerInfo(LEDGERHASH, 30);
EXPECT_CALL(*rawBackendPtr, fetchLedgerBySequence).Times(1);
ON_CALL(*rawBackendPtr, fetchLedgerBySequence).WillByDefault(Return(ledgerinfo));
auto const account = GetAccountIDWithString(ACCOUNT);
auto const accountKk = ripple::keylet::account(account).key;
auto const accountRoot = CreateAccountRootObject(ACCOUNT, 0, 2, 200, 2, INDEX1, 2);
ON_CALL(*rawBackendPtr, doFetchLedgerObject(accountKk, 30, _))
.WillByDefault(Return(accountRoot.getSerializer().peekData()));
auto signersKey = ripple::keylet::signers(account).key;
ON_CALL(*rawBackendPtr, doFetchLedgerObject(signersKey, 30, _))
.WillByDefault(Return(CreateSignerLists({{ACCOUNT1, 1}, {ACCOUNT2, 1}}).getSerializer().peekData()));
ON_CALL(*rawBackendPtr, doFetchLedgerObject(ripple::keylet::amendments().key, 30, _))
.WillByDefault(Return(CreateAmendmentsObject({}).getSerializer().peekData()));
EXPECT_CALL(*rawBackendPtr, doFetchLedgerObject).Times(4);
auto const static input = json::parse(fmt::format(
R"({{
"account": "{}",
"signer_lists": true
}})",
ACCOUNT));
auto const handler = AnyHandler{AccountInfoHandler{mockBackendPtr}};
runSpawn([&](auto yield) {
auto const output = handler.process(input, Context{.yield = yield, .apiVersion = 2});
ASSERT_TRUE(output);
EXPECT_EQ(*output, json::parse(expectedOutput));
});
}
TEST_F(RPCAccountInfoHandlerTest, SignerListsTrueV1)
{
auto const expectedOutput = fmt::format(
R"({{
"account_data":
{{
"Account": "{}",
"Balance": "200",
"Flags": 0,
"LedgerEntryType": "AccountRoot",
"OwnerCount": 2,
"PreviousTxnID": "{}",
"PreviousTxnLgrSeq": 2,
"Sequence": 2,
"TransferRate": 0,
"index": "13F1A95D7AAB7108D5CE7EEAF504B2894B8C674E6D68499076441C4837282BF8",
"signer_lists":
[
{{
"Flags": 0,
"LedgerEntryType": "SignerList",
"OwnerNode": "0",
"PreviousTxnID": "0000000000000000000000000000000000000000000000000000000000000000",
"PreviousTxnLgrSeq": 0,
"SignerEntries":
[
{{
"SignerEntry":
{{
"Account": "{}",
"SignerWeight": 1
}}
}},
{{
"SignerEntry":
{{
"Account": "{}",
"SignerWeight": 1
}}
}}
],
"SignerListID": 0,
"SignerQuorum": 2,
"index": "A9C28A28B85CD533217F5C0A0C7767666B093FA58A0F2D80026FCC4CD932DDC7"
}}
]
}},
"account_flags":
{{
"defaultRipple": false,
"depositAuth": false,
"disableMasterKey": false,
"disallowIncomingXRP": false,
"globalFreeze": false,
"noFreeze": false,
"passwordSpent": false,
"requireAuthorization": false,
"requireDestinationTag": false
}},
"ledger_hash": "{}",
"ledger_index": 30,
"validated": true
}})",
ACCOUNT,
INDEX1,
ACCOUNT1,
ACCOUNT2,
LEDGERHASH);
@@ -412,7 +481,7 @@ TEST_F(RPCAccountInfoHandlerTest, SignerListsTrue)
ACCOUNT));
auto const handler = AnyHandler{AccountInfoHandler{mockBackendPtr}};
runSpawn([&](auto yield) {
auto const output = handler.process(input, Context{yield});
auto const output = handler.process(input, Context{.yield = yield, .apiVersion = 1});
ASSERT_TRUE(output);
EXPECT_EQ(*output, json::parse(expectedOutput));
});

26
unittests/rpc/handlers/LedgerEntryTests.cpp
View File

@@ -1085,3 +1085,29 @@ TEST_F(RPCLedgerEntryTest, LedgerNotExistViaHash)
EXPECT_EQ(err.at("error_message").as_string(), "ledgerNotFound");
});
}
TEST_F(RPCLedgerEntryTest, InvalidEntryTypeVersion2)
{
runSpawn([&, this](auto yield) {
auto const handler = AnyHandler{LedgerEntryHandler{mockBackendPtr}};
auto const req = json::parse(R"({})");
auto const output = handler.process(req, Context{.yield = yield, .apiVersion = 2});
ASSERT_FALSE(output);
auto const err = rpc::makeError(output.error());
EXPECT_EQ(err.at("error").as_string(), "unknownOption");
EXPECT_EQ(err.at("error_message").as_string(), "Unknown option.");
});
}
TEST_F(RPCLedgerEntryTest, InvalidEntryTypeVersion1)
{
runSpawn([&, this](auto yield) {
auto const handler = AnyHandler{LedgerEntryHandler{mockBackendPtr}};
auto const req = json::parse(R"({})");
auto const output = handler.process(req, Context{.yield = yield, .apiVersion = 1});
ASSERT_FALSE(output);
auto const err = rpc::makeError(output.error());
EXPECT_EQ(err.at("error").as_string(), "invalidParams");
EXPECT_EQ(err.at("error_message").as_string(), "Invalid parameters.");
});
}