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

Amendment table clean up (Improve amendment processing and activation logic) #3428

Closed
wants to merge 12 commits into from
7 changes: 1 addition & 6 deletions src/ripple/app/main/Application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,6 @@

namespace ripple {

// 204/256 about 80%
static int const MAJORITY_FRACTION_OLD(204);
// 8/10 = 80%
static int const MAJORITY_FRACTION(8);

//------------------------------------------------------------------------------

namespace detail {
Expand Down Expand Up @@ -1536,7 +1531,7 @@ ApplicationImp::setup()

m_amendmentTable = make_AmendmentTable(
config().AMENDMENT_MAJORITY_TIME,
{MAJORITY_FRACTION_OLD, MAJORITY_FRACTION},
{amendmentMajorityFractionOld, amendmentMajorityFraction},
supportedAmendments,
enabledAmendments,
config_->section(SECTION_VETO_AMENDMENTS),
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/core/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class Config : public BasicConfig
bool COMPRESSION = false;

// Amendment majority time
std::chrono::seconds AMENDMENT_MAJORITY_TIME = weeks{2};
std::chrono::seconds AMENDMENT_MAJORITY_TIME = defaultAmendmentMajorityTime;

// Thread pool configuration
std::size_t WORKERS = 0;
Expand Down
15 changes: 10 additions & 5 deletions src/ripple/core/impl/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -485,24 +485,29 @@ Config::loadFromString(std::string const& fileContents)
{
using namespace std::chrono;
boost::regex const re(
"^\\s*(\\d+)\\s*(seconds|minutes|hours|days|weeks)\\s*(\\s+.*)?$");
"^\\s*(\\d+)\\s*(minutes|hours|days|weeks)\\s*(\\s+.*)?$");
boost::smatch match;
if (!boost::regex_match(strTemp, match, re))
Throw<std::runtime_error>(
"Invalid " SECTION_AMENDMENT_MAJORITY_TIME
", must be: [0-9]+ [seconds|minutes|hours|days|weeks]");
", must be: [0-9]+ [minutes|hours|days|weeks]");

std::uint32_t duration =
beast::lexicalCastThrow<std::uint32_t>(match[1].str());
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not allow seconds: there's no real plausible scenario for such a short interval. In fact, I think I'd reject any interval smaller than 15 minutes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

if (boost::iequals(match[2], "seconds"))
AMENDMENT_MAJORITY_TIME = seconds(duration);
else if (boost::iequals(match[2], "minutes"))

if (boost::iequals(match[2], "minutes"))
AMENDMENT_MAJORITY_TIME = minutes(duration);
else if (boost::iequals(match[2], "hours"))
AMENDMENT_MAJORITY_TIME = hours(duration);
else if (boost::iequals(match[2], "days"))
AMENDMENT_MAJORITY_TIME = days(duration);
else if (boost::iequals(match[2], "weeks"))
AMENDMENT_MAJORITY_TIME = weeks(duration);

if (AMENDMENT_MAJORITY_TIME < defaultAmendmentMajorityTime)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this check is wrong. It will prevent anything less than 2 weeks. I'd simply hard-code std::chrono::minutes(15) here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right!!!

Throw<std::runtime_error>(
"Invalid " SECTION_AMENDMENT_MAJORITY_TIME
", the minimum amount of time an amendment must hold a majority is 15 minutes");
}

// Do not load trusted validator configuration for standalone mode
Expand Down
13 changes: 13 additions & 0 deletions src/ripple/protocol/SystemParameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#define RIPPLE_PROTOCOL_SYSTEMPARAMETERS_H_INCLUDED

#include <ripple/basics/XRPAmount.h>
#include <ripple/basics/chrono.h>
nbougalis marked this conversation as resolved.
Show resolved Hide resolved
#include <cstdint>
#include <string>

Expand Down Expand Up @@ -59,6 +60,18 @@ systemCurrencyCode()
/** The XRP ledger network's earliest allowed sequence */
static std::uint32_t constexpr XRP_LEDGER_EARLIEST_SEQ{32570};

/** The minimum amount of support an amendment should have.

@note This value is used by legacy code and will become obsolete
once the fix3396 amendment activates.
*/
constexpr int const amendmentMajorityFractionOld = 204;

constexpr int const amendmentMajorityFraction = 8;

/** The minimum amount of time an amendment must hold a majority */
constexpr std::chrono::seconds const defaultAmendmentMajorityTime = weeks{2};

} // namespace ripple

#endif