Skip to content
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

Check amendment block status and update with ledgers: #2137

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Builds/VisualStudio2015/RippleD.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -4931,6 +4931,10 @@
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
</ClCompile>
<ClCompile Include="..\..\src\test\rpc\AmendmentBlocked_test.cpp">
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
</ClCompile>
<ClCompile Include="..\..\src\test\rpc\Book_test.cpp">
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
Expand Down
3 changes: 3 additions & 0 deletions Builds/VisualStudio2015/RippleD.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -5646,6 +5646,9 @@
<ClCompile Include="..\..\src\test\rpc\AccountSet_test.cpp">
<Filter>test\rpc</Filter>
</ClCompile>
<ClCompile Include="..\..\src\test\rpc\AmendmentBlocked_test.cpp">
<Filter>test\rpc</Filter>
</ClCompile>
<ClCompile Include="..\..\src\test\rpc\Book_test.cpp">
<Filter>test\rpc</Filter>
</ClCompile>
Expand Down
7 changes: 5 additions & 2 deletions src/ripple/app/consensus/RCLConsensus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -910,9 +910,12 @@ RCLConsensus::startRound(
RCLCxLedger::ID const& prevLgrId,
RCLCxLedger const& prevLgr)
{
// We have a key, and we have some idea what the ledger is
// We have a key, we have some idea what the ledger is, and we are not
// amendment blocked
validating_ =
!app_.getOPs().isNeedNetworkLedger() && (valPublic_.size() != 0);
!app_.getOPs().isNeedNetworkLedger() &&
(valPublic_.size() != 0) &&
!app_.getOPs().isAmendmentBlocked();

// propose only if we're in sync with the network (and validating)
bool proposing =
Expand Down
7 changes: 7 additions & 0 deletions src/ripple/app/ledger/impl/LedgerMaster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,13 @@ LedgerMaster::setValidLedger(
app_.getSHAMapStore().onLedgerClosed (getValidatedLedger());
mLedgerHistory.validatedLedger (l);
app_.getAmendmentTable().doValidatedLedger (l);
if (!app_.getOPs().isAmendmentBlocked() &&
app_.getAmendmentTable().hasUnsupportedEnabled ())
{
JLOG (m_journal.error()) <<
"One or more unsupported amendments activated: server blocked.";
app_.getOPs().setAmendmentBlocked();
}
}

void
Expand Down
8 changes: 8 additions & 0 deletions src/ripple/app/misc/AmendmentTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ class AmendmentTable
virtual bool isEnabled (uint256 const& amendment) = 0;
virtual bool isSupported (uint256 const& amendment) = 0;

/**
* @brief returns true if one or more amendments on the network
* have been enabled that this server does not support
*
* @return true if an unsupported feature is enabled on the network
*/
virtual bool hasUnsupportedEnabled () = 0;

virtual Json::Value getJson (int) = 0;

/** Returns a Json::objectValue. */
Expand Down
27 changes: 23 additions & 4 deletions src/ripple/app/misc/impl/AmendmentTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ class AmendmentTableImpl final
// we haven't participated in one yet.
std::unique_ptr <AmendmentSet> lastVote_;

// True if an unsupported amendment is enabled
bool unsupportedEnabled_;

beast::Journal j_;

// Finds or creates state
Expand Down Expand Up @@ -187,6 +190,8 @@ class AmendmentTableImpl final
bool isEnabled (uint256 const& amendment) override;
bool isSupported (uint256 const& amendment) override;

bool hasUnsupportedEnabled () override;

Json::Value getJson (int) override;
Json::Value getJson (uint256 const&) override;

Expand Down Expand Up @@ -222,6 +227,7 @@ AmendmentTableImpl::AmendmentTableImpl (
: lastUpdateSeq_ (0)
, majorityTime_ (majorityTime)
, majorityFraction_ (majorityFraction)
, unsupportedEnabled_ (false)
, j_ (journal)
{
assert (majorityFraction_ != 0);
Expand Down Expand Up @@ -340,6 +346,14 @@ AmendmentTableImpl::enable (uint256 const& amendment)
return false;

s->enabled = true;

if (! s->supported)
{
JLOG (j_.error()) <<
"Unsupported amendment " << amendment << " activated.";
unsupportedEnabled_ = true;
}

return true;
}

Expand Down Expand Up @@ -372,6 +386,13 @@ AmendmentTableImpl::isSupported (uint256 const& amendment)
return s && s->supported;
}

bool
AmendmentTableImpl::hasUnsupportedEnabled ()
{
std::lock_guard <std::mutex> sl (mutex_);
return unsupportedEnabled_;
}

std::vector <uint256>
AmendmentTableImpl::doValidation (
std::set<uint256> const& enabled)
Expand Down Expand Up @@ -523,10 +544,8 @@ AmendmentTableImpl::doValidatedLedger (
LedgerIndex ledgerSeq,
std::set<uint256> const& enabled)
{
std::lock_guard <std::mutex> sl (mutex_);

for (auto& e : amendmentMap_)
e.second.enabled = (enabled.count (e.first) != 0);
for (auto& e : enabled)
enable(e);
}

void
Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/ErrorCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ enum error_code_i
rpcHIGH_FEE,
rpcNOT_ENABLED,
rpcNOT_READY,
rpcAMENDMENT_BLOCKED,

// Networking
rpcNO_CLOSED,
Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/impl/ErrorCodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class ErrorCategory
add (rpcACT_EXISTS, "actExists", "Account already exists.");
add (rpcACT_MALFORMED, "actMalformed", "Account malformed.");
add (rpcACT_NOT_FOUND, "actNotFound", "Account not found.");
add (rpcAMENDMENT_BLOCKED, "amendmentBlocked", "Amendment blocked, need upgrade.");
add (rpcATX_DEPRECATED, "deprecated", "Use the new API or specify a ledger range.");
add (rpcBAD_BLOB, "badBlob", "Blob must be a non-empty hex string.");
add (rpcBAD_FEATURE, "badFeature", "Feature unknown or invalid.");
Expand Down
7 changes: 7 additions & 0 deletions src/ripple/rpc/impl/RPCHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,13 @@ error_code_i fillHandler (Context& context,
return rpcNO_NETWORK;
}

if (context.app.getOPs().isAmendmentBlocked() &&
(handler->condition_ & NEEDS_CURRENT_LEDGER ||
handler->condition_ & NEEDS_CLOSED_LEDGER))
{
return rpcAMENDMENT_BLOCKED;
}

if (!context.app.config().standalone() &&
handler->condition_ & NEEDS_CURRENT_LEDGER)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the NEEDS_CLOSED_LEDGER also only work if not amendment blocked? Note that I don't see any RPCs actually using that condition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I wondered that too and had a hard time deciding since, as you point out, there aren't any methods currently using it. I think I'll add it to the check as you suggest.

Copy link
Contributor

@wilsonianb wilsonianb Jun 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of using these conditions to determine what commands to reject when an amendment blocked.
At first, I was concerned about NO_CONDITION commands such as account_channels, account_objects, channel_authorize, channel_verify, and ledger_entry that are affected by amendments. But commands specific to an amendment would simply return an unknownCmd error, and when I reverted the commit that added PayChan, rippled crashed before I could even try looking up account objects/channels or ledger entries

Terminating thread doJob: OrderBookDB::update: unhandled St13runtime_error 'invalid ledger entry type'

All that to say, I think this change is a good addition to the existing limitations in rippled's ability to serve RPC requests while amendment blocked.

{
Expand Down
15 changes: 15 additions & 0 deletions src/test/app/AmendmentTable_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,20 @@ class AmendmentTable_test final : public beast::unit_test::suite
}
}

void testHasUnsupported ()
{
testcase ("hasUnsupportedEnabled");

auto table = makeTable(1);
BEAST_EXPECT(! table->hasUnsupportedEnabled());

std::set <uint256> enabled;
std::for_each(m_set4.begin(), m_set4.end(),
[&enabled](auto const &s){ enabled.insert(amendmentId(s)); });
table->doValidatedLedger(1, enabled);
BEAST_EXPECT(table->hasUnsupportedEnabled());
}

void run ()
{
testConstruct();
Expand All @@ -757,6 +771,7 @@ class AmendmentTable_test final : public beast::unit_test::suite
testDetectMajority ();
testLostMajority ();
testSupportedAmendments ();
testHasUnsupported ();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this

}
};

Expand Down
169 changes: 169 additions & 0 deletions src/test/rpc/AmendmentBlocked_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2017 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.
*/
//==============================================================================

#include <test/jtx.h>
#include <ripple/protocol/JsonFields.h>
#include <ripple/app/misc/NetworkOPs.h>
#include <test/jtx/WSClient.h>

namespace ripple {

class AmendmentBlocked_test : public beast::unit_test::suite
{
void testBlockedMethods()
{
using namespace test::jtx;
Env env {*this};
auto const gw = Account {"gateway"};
auto const USD = gw["USD"];
auto const alice = Account {"alice"};
auto const bob = Account {"bob"};
env.fund (XRP(10000), alice, bob, gw);
env.trust (USD(600), alice);
env.trust (USD(700), bob);
env(pay (gw, alice, USD(70)));
env(pay (gw, bob, USD(50)));
env.close();

auto wsc = test::makeWSClient(env.app().config());

auto current = env.current ();
// ledger_accept
auto jr = env.rpc ("ledger_accept") [jss::result];
BEAST_EXPECT (jr[jss::ledger_current_index] == current->seq ()+1);

// ledger_current
jr = env.rpc ("ledger_current") [jss::result];
BEAST_EXPECT (jr[jss::ledger_current_index] == current->seq ()+1);

// owner_info
jr = env.rpc ("owner_info", alice.human()) [jss::result];
BEAST_EXPECT (jr.isMember (jss::accepted) && jr.isMember (jss::current));

// path_find
Json::Value pf_req;
pf_req[jss::subcommand] = "create";
pf_req[jss::source_account] = alice.human();
pf_req[jss::destination_account] = bob.human();
pf_req[jss::destination_amount] = bob["USD"](20).value ().getJson (0);
jr = wsc->invoke("path_find", pf_req) [jss::result];
BEAST_EXPECT (jr.isMember (jss::alternatives) &&
jr[jss::alternatives].isArray () &&
jr[jss::alternatives].size () == 1);

// submit
auto jt = env.jt (noop (alice));
Serializer s;
jt.stx->add (s);
jr = env.rpc ("submit", strHex (s.slice ())) [jss::result];
BEAST_EXPECT (jr.isMember (jss::engine_result) &&
jr[jss::engine_result] == "tesSUCCESS");

// submit_multisigned
env(signers(bob, 1, {{alice, 1}}), sig (bob));
Account const ali {"ali", KeyType::secp256k1};
env(regkey (alice, ali));
env.close();

Json::Value set_tx;
set_tx[jss::Account] = bob.human();
set_tx[jss::TransactionType] = "AccountSet";
set_tx[jss::Fee] =
static_cast<uint32_t>(8 * env.current()->fees().base);
set_tx[jss::Sequence] = env.seq(bob);
set_tx[jss::SigningPubKey] = "";

Json::Value sign_for;
sign_for[jss::tx_json] = set_tx;
sign_for[jss::account] = alice.human();
sign_for[jss::secret] = ali.name();
jr = env.rpc("json", "sign_for", to_string(sign_for)) [jss::result];
BEAST_EXPECT(jr[jss::status] == "success");

Json::Value ms_req;
ms_req[jss::tx_json] = jr[jss::tx_json];
jr = env.rpc("json", "submit_multisigned", to_string(ms_req))
[jss::result];
BEAST_EXPECT (jr.isMember (jss::engine_result) &&
jr[jss::engine_result] == "tesSUCCESS");

// make the network amendment blocked...now all the same
// requests should fail

env.app ().getOPs ().setAmendmentBlocked ();

// ledger_accept
jr = env.rpc ("ledger_accept") [jss::result];
BEAST_EXPECT(
jr.isMember (jss::error) &&
jr[jss::error] == "amendmentBlocked");
BEAST_EXPECT(jr[jss::status] == "error");

// ledger_current
jr = env.rpc ("ledger_current") [jss::result];
BEAST_EXPECT(
jr.isMember (jss::error) &&
jr[jss::error] == "amendmentBlocked");
BEAST_EXPECT(jr[jss::status] == "error");

// owner_info
jr = env.rpc ("owner_info", alice.human()) [jss::result];
BEAST_EXPECT(
jr.isMember (jss::error) &&
jr[jss::error] == "amendmentBlocked");
BEAST_EXPECT(jr[jss::status] == "error");

// path_find
jr = wsc->invoke("path_find", pf_req) [jss::result];
BEAST_EXPECT(
jr.isMember (jss::error) &&
jr[jss::error] == "amendmentBlocked");
BEAST_EXPECT(jr[jss::status] == "error");

// submit
jr = env.rpc("submit", strHex(s.slice())) [jss::result];
BEAST_EXPECT(
jr.isMember (jss::error) &&
jr[jss::error] == "amendmentBlocked");
BEAST_EXPECT(jr[jss::status] == "error");

// submit_multisigned
set_tx[jss::Sequence] = env.seq(bob);
sign_for[jss::tx_json] = set_tx;
jr = env.rpc("json", "sign_for", to_string(sign_for)) [jss::result];
BEAST_EXPECT(jr[jss::status] == "success");
ms_req[jss::tx_json] = jr[jss::tx_json];
jr = env.rpc("json", "submit_multisigned", to_string(ms_req))
[jss::result];
BEAST_EXPECT(
jr.isMember (jss::error) &&
jr[jss::error] == "amendmentBlocked");
}

public:
void run()
{
testBlockedMethods();
}
};

BEAST_DEFINE_TESTSUITE(AmendmentBlocked,app,ripple);

}

Loading