Skip to content

Commit

Permalink
Remove conditionals for featureMultiSign enabled 27Jun2016
Browse files Browse the repository at this point in the history
  • Loading branch information
scottschurr authored and manojsdoshi committed Jan 30, 2020
1 parent c48be14 commit 6e4945c
Show file tree
Hide file tree
Showing 14 changed files with 21 additions and 116 deletions.
7 changes: 1 addition & 6 deletions src/ripple/app/tx/impl/SetAccount.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,12 +295,7 @@ SetAccount::doApply ()
(!view().peek (keylet::signers (account_))))
{
// Account has no regular key or multi-signer signer list.

// Prevent transaction changes until we're ready.
if (view().rules().enabled(featureMultiSign))
return tecNO_ALTERNATIVE_KEY;

return tecNO_REGULAR_KEY;
return tecNO_ALTERNATIVE_KEY;
}

JLOG(j_.trace()) << "Set lsfDisableMaster.";
Expand Down
3 changes: 0 additions & 3 deletions src/ripple/app/tx/impl/SetSignerList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ SetSignerList::determineOperation(STTx const& tx,
NotTEC
SetSignerList::preflight (PreflightContext const& ctx)
{
if (! ctx.rules.enabled(featureMultiSign))
return temDISABLED;

auto const ret = preflight1 (ctx);
if (!isTesSuccess (ret))
return ret;
Expand Down
10 changes: 3 additions & 7 deletions src/ripple/app/tx/impl/Transactor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -331,13 +331,9 @@ TER Transactor::apply ()
NotTEC
Transactor::checkSign (PreclaimContext const& ctx)
{
// Make sure multisigning is enabled before we check for multisignatures.
if (ctx.view.rules().enabled(featureMultiSign))
{
// If the pk is empty, then we must be multi-signing.
if (ctx.tx.getSigningPubKey().empty ())
return checkMultiSign (ctx);
}
// If the pk is empty, then we must be multi-signing.
if (ctx.tx.getSigningPubKey().empty ())
return checkMultiSign (ctx);

return checkSingleSign (ctx);
}
Expand Down
5 changes: 1 addition & 4 deletions src/ripple/app/tx/impl/apply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ checkValidity(HashRouter& router,
STTx const& tx, Rules const& rules,
Config const& config)
{
auto const allowMultiSign =
rules.enabled(featureMultiSign);

auto const id = tx.getTransactionID();
auto const flags = router.getFlags(id);
if (flags & SF_SIGBAD)
Expand All @@ -50,7 +47,7 @@ checkValidity(HashRouter& router,
if (!(flags & SF_SIGGOOD))
{
// Don't know signature state. Check it.
auto const sigVerify = tx.checkSign(allowMultiSign);
auto const sigVerify = tx.checkSign();
if (! sigVerify.first)
{
router.setFlags(id, SF_SIGBAD);
Expand Down
3 changes: 1 addition & 2 deletions src/ripple/protocol/Feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class FeatureCollections
{
static constexpr char const* const featureNames[] =
{
"MultiSign",
"MultiSign", // Unconditionally supported.
"Tickets",
"TrustSetAuth",
"FeeEscalation", // Unconditionally supported.
Expand Down Expand Up @@ -339,7 +339,6 @@ foreachFeature(FeatureBitset bs, F&& f)
f(bitsetIndexToFeature(i));
}

extern uint256 const featureMultiSign;
extern uint256 const featureTickets;
extern uint256 const featureTrustSetAuth;
extern uint256 const featureOwnerPaysFee;
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/protocol/STTx.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ class STTx final
@return `true` if valid signature. If invalid, the error message string.
*/
std::pair<bool, std::string>
checkSign(bool allowMultiSign) const;
checkSign() const;

// SQL Functions with metadata.
static
Expand Down
3 changes: 1 addition & 2 deletions src/ripple/protocol/impl/Feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ detail::supportedAmendments ()
// Removing them will cause servers to become amendment blocked.
static std::vector<std::string> const supported
{
"MultiSign",
"MultiSign", // Unconditionally supported.
// "Tickets",
"TrustSetAuth",
"FeeEscalation", // Unconditionally supported.
Expand Down Expand Up @@ -151,7 +151,6 @@ uint256 bitsetIndexToFeature(size_t i)
}


uint256 const featureMultiSign = *getRegisteredFeature("MultiSign");
uint256 const featureTickets = *getRegisteredFeature("Tickets");
uint256 const featureTrustSetAuth = *getRegisteredFeature("TrustSetAuth");
uint256 const featureOwnerPaysFee = *getRegisteredFeature("OwnerPaysFee");
Expand Down
20 changes: 6 additions & 14 deletions src/ripple/protocol/impl/STTx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,24 +177,16 @@ void STTx::sign (
tid_ = getHash(HashPrefix::transactionID);
}

std::pair<bool, std::string> STTx::checkSign(bool allowMultiSign) const
std::pair<bool, std::string> STTx::checkSign() const
{
std::pair<bool, std::string> ret {false, ""};
try
{
if (allowMultiSign)
{
// Determine whether we're single- or multi-signing by looking
// at the SigningPubKey. It it's empty we must be
// multi-signing. Otherwise we're single-signing.
Blob const& signingPubKey = getFieldVL (sfSigningPubKey);
ret = signingPubKey.empty () ?
checkMultiSign () : checkSingleSign ();
}
else
{
ret = checkSingleSign ();
}
// Determine whether we're single- or multi-signing by looking
// at the SigningPubKey. It it's empty we must be
// multi-signing. Otherwise we're single-signing.
Blob const& signingPubKey = getFieldVL (sfSigningPubKey);
ret = signingPubKey.empty () ? checkMultiSign () : checkSingleSign ();
}
catch (std::exception const&)
{
Expand Down
7 changes: 0 additions & 7 deletions src/ripple/rpc/handlers/SignFor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,6 @@ Json::Value doSignFor (RPC::JsonContext& context)
"Signing is not supported by this server.");
}

// Bail if multisign is not enabled.
if (! context.app.getLedgerMaster().getValidatedRules().
enabled (featureMultiSign))
{
RPC::inject_error (rpcNOT_ENABLED, context.params);
return context.params;
}
context.loadType = Resource::feeHighBurdenRPC;
auto const failHard = context.params[jss::fail_hard].asBool();
auto const failType = NetworkOPs::doFailHard (failHard);
Expand Down
7 changes: 0 additions & 7 deletions src/ripple/rpc/handlers/SubmitMultiSigned.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,6 @@ namespace ripple {
// }
Json::Value doSubmitMultiSigned (RPC::JsonContext& context)
{
// Bail if multisign is not enabled.
if (! context.app.getLedgerMaster().getValidatedRules().
enabled (featureMultiSign))
{
RPC::inject_error (rpcNOT_ENABLED, context.params);
return context.params;
}
context.loadType = Resource::feeHighBurdenRPC;
auto const failHard = context.params[jss::fail_hard].asBool();
auto const failType = NetworkOPs::doFailHard (failHard);
Expand Down
4 changes: 2 additions & 2 deletions src/test/app/Check_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,7 @@ class Check_test : public beast::unit_test::suite
}

// Use a regular key and also multisign to cash a check.
// featureMultiSign changes the reserve on a SignerList, so
// featureMultiSignReserve changes the reserve on a SignerList, so
// check both before and after.
FeatureBitset const allSupported {supported_amendments()};
for (auto const features :
Expand Down Expand Up @@ -1541,7 +1541,7 @@ class Check_test : public beast::unit_test::suite
Account const zoe {"zoe"};
IOU const USD {gw["USD"]};

// featureMultiSign changes the reserve on a SignerList, so
// featureMultiSignReserve changes the reserve on a SignerList, so
// check both before and after.
FeatureBitset const allSupported {supported_amendments()};
for (auto const features :
Expand Down
51 changes: 0 additions & 51 deletions src/test/app/MultiSign_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,56 +204,6 @@ class MultiSign_test : public beast::unit_test::suite
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
}

void
test_enablement (FeatureBitset features)
{
testcase ("Enablement");

using namespace jtx;
Env env(*this, envconfig([](std::unique_ptr<Config> cfg)
{
cfg->loadFromString ("[" SECTION_SIGNING_SUPPORT "]\ntrue");
return cfg;
}), features - featureMultiSign);

Account const alice {"alice", KeyType::ed25519};
env.fund(XRP(1000), alice);
env.close();

// NOTE: These six tests will fail if multisign is enabled.
env(signers(alice, 1, {{bogie, 1}}), ter(temDISABLED));
env.close();
env.require (owners (alice, 0));

std::uint32_t aliceSeq = env.seq (alice);
auto const baseFee = env.current()->fees().base;
env(noop(alice), msig(bogie), fee(2 * baseFee), ter(temINVALID));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq);

env(signers(alice, 1, {{bogie, 1}, {demon,1}}), ter(temDISABLED));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq);

{
Json::Value jvParams;
jvParams[jss::account] = alice.human();
auto const jsmr = env.rpc("json", "submit_multisigned", to_string(jvParams))[jss::result];
BEAST_EXPECT(jsmr[jss::error] == "notEnabled");
BEAST_EXPECT(jsmr[jss::status] == "error");
BEAST_EXPECT(jsmr[jss::error_message] == "Not enabled in configuration.");
}

{
Json::Value jvParams;
jvParams[jss::account] = alice.human();
auto const jsmr = env.rpc("json", "sign_for", to_string(jvParams))[jss::result];
BEAST_EXPECT(jsmr[jss::error] == "notEnabled");
BEAST_EXPECT(jsmr[jss::status] == "error");
BEAST_EXPECT(jsmr[jss::error_message] == "Not enabled in configuration.");
}
}

void test_fee (FeatureBitset features)
{
testcase ("Fee");
Expand Down Expand Up @@ -1340,7 +1290,6 @@ class MultiSign_test : public beast::unit_test::suite
test_noReserve (features);
test_signerListSet (features);
test_phantomSigners (features);
test_enablement (features);
test_fee (features);
test_misorderedSigners (features);
test_masterSigners (features);
Expand Down
2 changes: 1 addition & 1 deletion src/test/protocol/STTx_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1478,7 +1478,7 @@ class STTx_test : public beast::unit_test::suite
});
j.sign (keypair.first, keypair.second);

unexpected (!j.checkSign (true).first, "Transaction fails signature test");
unexpected (!j.checkSign().first, "Transaction fails signature test");

Serializer rawTxn;
j.add (rawTxn);
Expand Down
13 changes: 4 additions & 9 deletions src/test/rpc/AccountSet_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -376,12 +376,10 @@ class AccountSet_test : public beast::unit_test::suite
}
}

void testBadInputs(bool withFeatures)
void testBadInputs()
{
using namespace test::jtx;
std::unique_ptr<Env> penv {
withFeatures ? new Env(*this) : new Env(*this, FeatureBitset{})};
Env& env = *penv;
Env env (*this);
Account const alice ("alice");
env.fund(XRP(10000), alice);

Expand Down Expand Up @@ -415,7 +413,7 @@ class AccountSet_test : public beast::unit_test::suite

env(fset (alice, asfDisableMaster),
sig(alice),
ter(withFeatures ? tecNO_ALTERNATIVE_KEY : tecNO_REGULAR_KEY));
ter(tecNO_ALTERNATIVE_KEY));
}

void testRequireAuthWithDir()
Expand Down Expand Up @@ -450,13 +448,10 @@ class AccountSet_test : public beast::unit_test::suite
testMessageKey();
testWalletID();
testEmailHash();
testBadInputs(true);
testBadInputs(false);
testBadInputs();
testRequireAuthWithDir();
testTransferRate();
}


};

BEAST_DEFINE_TESTSUITE(AccountSet,app,ripple);
Expand Down

0 comments on commit 6e4945c

Please sign in to comment.