Remove full from ledger RPC forwarding (#1360)

Fixes #777
This commit is contained in:
Alex Kremer
2024-04-15 18:53:14 +01:00
committed by GitHub
parent e931f27d3b
commit e66cc7759e
5 changed files with 48 additions and 131 deletions

View File

@@ -121,7 +121,7 @@ public:
verify(boost::json::value const& value, std::string_view key)
{
if (value.is_object() and value.as_object().contains(key.data()))
return Error{Status{RippledError::rpcNOT_SUPPORTED, "Not supported field '" + std::string{key}}};
return Error{Status{RippledError::rpcNOT_SUPPORTED, "Not supported field '" + std::string{key} + '\''}};
return {};
}

View File

@@ -75,11 +75,8 @@ public:
};
auto const checkLedgerForward = [&]() {
return ctx.method == "ledger" and
((request.contains("queue") and request.at("queue").is_bool() and request.at("queue").as_bool()) or
(request.contains("full") and request.at("full").is_bool() and request.at("full").as_bool()) or
(request.contains("accounts") and request.at("accounts").is_bool() and request.at("accounts").as_bool()
));
return ctx.method == "ledger" and request.contains("queue") and request.at("queue").is_bool() and
request.at("queue").as_bool();
};
return static_cast<bool>(checkAccountInfoForward() or checkLedgerForward());

View File

@@ -62,13 +62,16 @@ public:
* @brief A struct to hold the input data for the command
*
* Clio does not support:
* - accounts
* - full
* - owner_finds
* - queue
*
* And the following are deprecated altogether:
* - full
* - accounts
* - ledger
* - type
*
* Clio will throw an error when any of `accounts`/`full`/`owner_funds`/`queue` are set to `true`
* Clio will throw an error when `queue` is set to `true`
* or if `full` or `accounts` are used.
* @see https://github.com/XRPLF/clio/issues/603
*/
struct Input {
@@ -102,9 +105,9 @@ public:
spec([[maybe_unused]] uint32_t apiVersion)
{
static auto const rpcSpec = RpcSpec{
{JS(full), validation::Type<bool>{}, validation::NotSupported{true}},
{JS(full), validation::NotSupported{}},
{JS(full), check::Deprecated{}},
{JS(accounts), validation::Type<bool>{}, validation::NotSupported{true}},
{JS(accounts), validation::NotSupported{}},
{JS(accounts), check::Deprecated{}},
{JS(owner_funds), validation::Type<bool>{}},
{JS(queue), validation::Type<bool>{}, validation::NotSupported{true}},

View File

@@ -160,66 +160,6 @@ TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsTrueIfAccountInfoWithQueueSpe
});
}
TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsTrueIfLedgerWithQueueSpecified)
{
auto const rawHandlerProviderPtr = handlerProvider.get();
auto const apiVersion = 2u;
auto const method = "ledger";
auto const params = json::parse(R"({"queue": true})");
ON_CALL(*rawHandlerProviderPtr, isClioOnly(_)).WillByDefault(Return(false));
EXPECT_CALL(*rawHandlerProviderPtr, isClioOnly(method)).Times(1);
runSpawn([&](auto yield) {
auto const range = backend->fetchLedgerRange();
auto const ctx =
web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP, true);
auto const res = proxy.shouldForward(ctx);
ASSERT_TRUE(res);
});
}
TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsTrueIfLedgerWithFullSpecified)
{
auto const rawHandlerProviderPtr = handlerProvider.get();
auto const apiVersion = 2u;
auto const method = "ledger";
auto const params = json::parse(R"({"full": true})");
ON_CALL(*rawHandlerProviderPtr, isClioOnly(_)).WillByDefault(Return(false));
EXPECT_CALL(*rawHandlerProviderPtr, isClioOnly(method)).Times(1);
runSpawn([&](auto yield) {
auto const range = backend->fetchLedgerRange();
auto const ctx =
web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP, true);
auto const res = proxy.shouldForward(ctx);
ASSERT_TRUE(res);
});
}
TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsTrueIfLedgerWithAccountsSpecified)
{
auto const rawHandlerProviderPtr = handlerProvider.get();
auto const apiVersion = 2u;
auto const method = "ledger";
auto const params = json::parse(R"({"accounts": true})");
ON_CALL(*rawHandlerProviderPtr, isClioOnly(_)).WillByDefault(Return(false));
EXPECT_CALL(*rawHandlerProviderPtr, isClioOnly(method)).Times(1);
runSpawn([&](auto yield) {
auto const range = backend->fetchLedgerRange();
auto const ctx =
web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP, true);
auto const res = proxy.shouldForward(ctx);
ASSERT_TRUE(res);
});
}
TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsFalseIfAccountInfoQueueIsFalse)
{
auto const rawHandlerProviderPtr = handlerProvider.get();
@@ -240,6 +180,26 @@ TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsFalseIfAccountInfoQueueIsFals
});
}
TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsTrueIfLedgerWithQueueSpecified)
{
auto const rawHandlerProviderPtr = handlerProvider.get();
auto const apiVersion = 2u;
auto const method = "ledger";
auto const params = json::parse(R"({"queue": true})");
ON_CALL(*rawHandlerProviderPtr, isClioOnly(_)).WillByDefault(Return(false));
EXPECT_CALL(*rawHandlerProviderPtr, isClioOnly(method)).Times(1);
runSpawn([&](auto yield) {
auto const range = backend->fetchLedgerRange();
auto const ctx =
web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP, true);
auto const res = proxy.shouldForward(ctx);
ASSERT_TRUE(res);
});
}
TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsFalseIfLedgerQueueIsFalse)
{
auto const rawHandlerProviderPtr = handlerProvider.get();
@@ -260,46 +220,6 @@ TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsFalseIfLedgerQueueIsFalse)
});
}
TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsFalseIfLedgerFullIsFalse)
{
auto const rawHandlerProviderPtr = handlerProvider.get();
auto const apiVersion = 2u;
auto const method = "ledger";
auto const params = json::parse(R"({"full": false})");
ON_CALL(*rawHandlerProviderPtr, isClioOnly(_)).WillByDefault(Return(false));
EXPECT_CALL(*rawHandlerProviderPtr, isClioOnly(method)).Times(1);
runSpawn([&](auto yield) {
auto const range = backend->fetchLedgerRange();
auto const ctx =
web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP, true);
auto const res = proxy.shouldForward(ctx);
ASSERT_FALSE(res);
});
}
TEST_F(RPCForwardingProxyTest, ShouldForwardReturnsFalseIfLedgerAccountsIsFalse)
{
auto const rawHandlerProviderPtr = handlerProvider.get();
auto const apiVersion = 2u;
auto const method = "ledger";
auto const params = json::parse(R"({"accounts": false})");
ON_CALL(*rawHandlerProviderPtr, isClioOnly(_)).WillByDefault(Return(false));
EXPECT_CALL(*rawHandlerProviderPtr, isClioOnly(method)).Times(1);
runSpawn([&](auto yield) {
auto const range = backend->fetchLedgerRange();
auto const ctx =
web::Context(yield, method, apiVersion, params.as_object(), nullptr, tagFactory, *range, CLIENT_IP, true);
auto const res = proxy.shouldForward(ctx);
ASSERT_FALSE(res);
});
}
TEST_F(RPCForwardingProxyTest, ShouldNotForwardReturnsTrueIfAPIVersionIsV1)
{
auto const apiVersion = 1u;

View File

@@ -85,28 +85,28 @@ generateTestValuesForParametersTest()
{
return std::vector<LedgerParamTestCaseBundle>{
{
"AccountsNotBool",
R"({"accounts": 123})",
"invalidParams",
"Invalid parameters.",
},
{
"AccountsInvalid",
"AccountsInvalidBool",
R"({"accounts": true})",
"notSupported",
"Not supported field 'accounts's value 'true'",
"Not supported field 'accounts'",
},
{
"FullExist",
"AccountsInvalidInt",
R"({"accounts": 123})",
"notSupported",
"Not supported field 'accounts'",
},
{
"FullInvalidBool",
R"({"full": true})",
"notSupported",
"Not supported field 'full's value 'true'",
"Not supported field 'full'",
},
{
"FullNotBool",
"FullInvalidInt",
R"({"full": 123})",
"invalidParams",
"Invalid parameters.",
"notSupported",
"Not supported field 'full'",
},
{
"QueueExist",
@@ -301,21 +301,18 @@ TEST_F(RPCLedgerHandlerTest, Default)
});
}
// not supported fields can be set to its default value
TEST_F(RPCLedgerHandlerTest, NotSupportedFieldsDefaultValue)
// fields not supported for specific value can be set to its default value
TEST_F(RPCLedgerHandlerTest, ConditionallyNotSupportedFieldsDefaultValue)
{
backend->setRange(RANGEMIN, RANGEMAX);
auto const ledgerinfo = CreateLedgerInfo(LEDGERHASH, RANGEMAX);
EXPECT_CALL(*backend, fetchLedgerBySequence).Times(1);
ON_CALL(*backend, fetchLedgerBySequence(RANGEMAX, _)).WillByDefault(Return(ledgerinfo));
EXPECT_CALL(*backend, fetchLedgerBySequence(RANGEMAX, _)).WillRepeatedly(Return(ledgerinfo));
runSpawn([&, this](auto yield) {
auto const handler = AnyHandler{LedgerHandler{backend}};
auto const req = json::parse(
R"({
"full": false,
"accounts": false,
"queue": false
})"
);