Allow an account to be exclusively multisigned (RIPD-182):

An account can be made signable with only its regular key by
disabling the master key.  Now an account can also be made
exclusively multisigned by both disabling the master key and
having no regular key.

In order to prevent an account from becoming unsignable the
network uses these rules:

 o An account can always add or replace a regular key or a
   SignerList as long as the fee and reserve can be met by the
   account.

 o The master key on an account can be disabled if either a
   regular key or a SignerList (or both) is present on the account.
   Either the regular key or the SignerList can be used to
   re-enable the master key later if that is desired.

 o The regular key on an account may only be removed if either the
   master key is enabled or the account has a SignerList (or both).

 o The SignerList on an account may only be removed if either the
   master key is enabled or a regular key is present (or both).

As a consequence of this change, the tecMASTER_DISABLED error
code is renamed to tecNO_ALTERNATIVE_KEY.  The error code number
(130 decimal) is unchanged.
This commit is contained in:
Scott Schurr
2015-08-06 14:52:27 -07:00
parent 6d2e3da306
commit f1c29ae20b
8 changed files with 162 additions and 70 deletions

View File

@@ -527,6 +527,81 @@ public:
env.require (owners (alice, 0));
}
// We want to always leave an account signable. Make sure the that we
// disallow removing the last way a transaction may be signed.
void test_keyDisable()
{
using namespace jtx;
Env env(*this);
Account const alice {"alice", KeyType::ed25519};
env.fund(XRP(1000), alice);
// There are three negative tests we need to make:
// M0. A lone master key cannot be disabled.
// R0. A lone regular key cannot be removed.
// L0. A lone signer list cannot be removed.
//
// Additionally, there are 6 positive tests we need to make:
// M1. The master key can be disabled if there's a regular key.
// M2. The master key can be disabled if there's a signer list.
//
// R1. The regular key can be removed if there's a signer list.
// R2. The regular key can be removed if the master key is enabled.
//
// L1. The signer list can be removed if the master key is enabled.
// L2. The signer list can be removed if there's a regular key.
// Master key tests.
// M0: A lone master key cannot be disabled.
env(fset (alice, asfDisableMaster),
sig(alice), ter(tecNO_ALTERNATIVE_KEY));
// Add a regular key.
Account const alie {"alie", KeyType::ed25519};
env(regkey (alice, alie));
// M1: The master key can be disabled if there's a regular key.
env(fset (alice, asfDisableMaster), sig(alice));
// R0: A lone regular key cannot be removed.
env(regkey (alice, disabled), sig(alie), ter(tecNO_ALTERNATIVE_KEY));
// Add a signer list.
env(signers(alice, 1, {{bogie, 1}}), sig (alie));
// R1: The regular key can be removed if there's a signer list.
env(regkey (alice, disabled), sig(alie));
// L0; A lone signer list cannot be removed.
auto const baseFee = env.config.FEE_DEFAULT;
env(signers(alice, jtx::none), msig(bogie),
fee(2 * baseFee), ter(tecNO_ALTERNATIVE_KEY));
// Enable the master key.
env(fclear (alice, asfDisableMaster), msig(bogie), fee(2 * baseFee));
// L1: The signer list can be removed if the master key is enabled.
env(signers(alice, jtx::none), msig(bogie), fee(2 * baseFee));
// Add a signer list.
env(signers(alice, 1, {{bogie, 1}}), sig (alice));
// M2: The master key can be disabled if there's a signer list.
env(fset (alice, asfDisableMaster), sig(alice));
// Add a regular key.
env(regkey (alice, alie), msig(bogie), fee(2 * baseFee));
// L2: The signer list can be removed if there's a regular key.
env(signers(alice, jtx::none), sig(alie));
// Enable the master key.
env(fclear (alice, asfDisableMaster), sig(alie));
// R2: The regular key can be removed if the master key is enabled.
env(regkey (alice, disabled), sig(alie));
}
// See if every kind of transaction can be successfully multi-signed.
void test_txTypes()
{
@@ -621,6 +696,7 @@ public:
test_masterSigners();
test_regularSigners();
test_heterogeneousSigners();
test_keyDisable();
test_txTypes();
}
};

View File

@@ -191,8 +191,18 @@ SetAccount::doApply ()
return tecNEED_MASTER_KEY;
}
if (!sle->isFieldPresent (sfRegularKey))
if ((!sle->isFieldPresent (sfRegularKey)) &&
(!view().peek (keylet::signers (account_))))
{
// Account has no regular key or multi-signer signer list.
// Prevent transaction changes until we're ready.
if ((RIPPLE_ENABLE_MULTI_SIGN) ||
view().flags() & tapENABLE_TESTING)
return tecNO_ALTERNATIVE_KEY;
return tecNO_REGULAR_KEY;
}
j_.trace << "Set lsfDisableMaster.";
uFlagsOut |= lsfDisableMaster;

View File

@@ -77,8 +77,11 @@ SetRegularKey::doApply ()
}
else
{
if (sle->isFlag (lsfDisableMaster))
return tecMASTER_DISABLED;
if (sle->isFlag (lsfDisableMaster) &&
!view().peek (keylet::signers (account_)))
// Account has disabled master key and no multi-signer signer list.
return tecNO_ALTERNATIVE_KEY;
sle->makeFieldAbsent (sfRegularKey);
}

View File

@@ -45,7 +45,7 @@ SetSignerList::determineOperation(STTx const& tx,
{
// Check the quorum. A non-zero quorum means we're creating or replacing
// the list. A zero quorum means we're destroying the list.
auto const quorum = tx.getFieldU32(sfSignerQuorum);
auto const quorum = tx[sfSignerQuorum];
std::vector<SignerEntries::SignerEntry> sign;
Operation op = unknown;
@@ -68,8 +68,7 @@ SetSignerList::determineOperation(STTx const& tx,
op = destroy;
}
return std::make_tuple(tesSUCCESS,
quorum, sign, op);
return std::make_tuple(tesSUCCESS, quorum, sign, op);
}
TER
@@ -115,21 +114,16 @@ SetSignerList::preflight (PreflightContext const& ctx)
TER
SetSignerList::doApply ()
{
// All operations require our ledger index. Compute that once and pass it
// to our handlers.
uint256 const index = getSignerListIndex (account_);
// Perform the operation preCompute() decided on.
switch (do_)
{
case set:
return replaceSignerList (index);
return replaceSignerList ();
case destroy:
return destroySignerList (index);
return destroySignerList ();
default:
// Fall through intentionally
break;
}
assert (false); // Should not be possible to get here.
@@ -210,24 +204,27 @@ SetSignerList::validateQuorumAndSignerEntries (
}
TER
SetSignerList::replaceSignerList (uint256 const& index)
SetSignerList::replaceSignerList ()
{
// This may be either a create or a replace. Preemptively destroy any
auto const accountKeylet = keylet::account (account_);
auto const ownerDirKeylet = keylet::ownerDir (account_);
auto const signerListKeylet = keylet::signers (account_);
// This may be either a create or a replace. Preemptively remove any
// old signer list. May reduce the reserve, so this is done before
// checking the reserve.
if (TER const ter = destroySignerList (index))
return ter;
if (TER const ter = removeSignersFromLedger (
accountKeylet, ownerDirKeylet, signerListKeylet))
return ter;
auto const sle = view().peek(
keylet::account(account_));
auto const sle = view().peek(accountKeylet);
// Compute new reserve. Verify the account has funds to meet the reserve.
std::uint32_t const oldOwnerCount = sle->getFieldU32 (sfOwnerCount);
auto const oldOwnerCount = (*sle)[sfOwnerCount];
std::uint32_t const addedOwnerCount = ownerCountDelta (signers_.size ());
auto const newReserve =
view().fees().accountReserve(
oldOwnerCount + addedOwnerCount);
view().fees().accountReserve(oldOwnerCount + addedOwnerCount);
// We check the reserve against the starting balance because we want to
// allow dipping into the reserve to pay fees. This behavior is consistent
@@ -236,20 +233,14 @@ SetSignerList::replaceSignerList (uint256 const& index)
return tecINSUFFICIENT_RESERVE;
// Everything's ducky. Add the ltSIGNER_LIST to the ledger.
auto signerList = std::make_shared<SLE>(ltSIGNER_LIST, index);
auto signerList = std::make_shared<SLE>(signerListKeylet);
view().insert (signerList);
writeSignersToLedger (signerList);
// Lambda for call to dirAdd.
auto describer = [&] (SLE::ref sle, bool dummy)
{
ownerDirDescriber (sle, dummy, account_);
};
writeSignersToSLE (signerList);
// Add the signer list to the account's directory.
std::uint64_t hint;
TER result = dirAdd(ctx_.view (),
hint, getOwnerDirIndex (account_), index, describer);
TER result = dirAdd(ctx_.view (), hint, ownerDirKeylet.key,
signerListKeylet.key, describeOwnerDir (account_));
JLOG(j_.trace) << "Create signer list for account " <<
toBase58(account_) << ": " << transHuman (result);
@@ -260,57 +251,60 @@ SetSignerList::replaceSignerList (uint256 const& index)
signerList->setFieldU64 (sfOwnerNode, hint);
// If we succeeded, the new entry counts against the creator's reserve.
adjustOwnerCount(view(),
sle, addedOwnerCount);
adjustOwnerCount(view(), sle, addedOwnerCount);
return result;
}
TER
SetSignerList::destroySignerList (uint256 const& index)
SetSignerList::destroySignerList ()
{
// See if there's an ltSIGNER_LIST for this account.
SLE::pointer signerList =
view().peek (keylet::signers(index));
auto const accountKeylet = keylet::account (account_);
// Destroying the signer list is only allowed if either the master key
// is enabled or there is a regular key.
SLE::pointer ledgerEntry = view().peek (accountKeylet);
if ((ledgerEntry->isFlag (lsfDisableMaster)) &&
(!ledgerEntry->isFieldPresent (sfRegularKey)))
return tecNO_ALTERNATIVE_KEY;
// If the signer list doesn't exist we've already succeeded in deleting it.
if (!signerList)
return tesSUCCESS;
auto const ownerDirKeylet = keylet::ownerDir (account_);
auto const signerListKeylet = keylet::signers (account_);
return removeSignersFromLedger(
accountKeylet, ownerDirKeylet, signerListKeylet);
}
TER
SetSignerList::removeSignersFromLedger (Keylet const& accountKeylet,
Keylet const& ownerDirKeylet, Keylet const& signerListKeylet)
{
// We have to examine the current SignerList so we know how much to
// reduce the OwnerCount.
std::int32_t removeFromOwnerCount = 0;
auto const k = keylet::signers(account_);
SLE::pointer accountSignersList =
view().peek (k);
if (accountSignersList)
{
STArray const& actualList =
accountSignersList->getFieldArray (sfSignerEntries);
removeFromOwnerCount = ownerCountDelta (actualList.size ()) * -1;
}
SLE::pointer signers = view().peek (signerListKeylet);
// If the signer list doesn't exist we've already succeeded in deleting it.
if (!signers)
return tesSUCCESS;
STArray const& actualList = signers->getFieldArray (sfSignerEntries);
int const removeFromOwnerCount = ownerCountDelta (actualList.size()) * -1;
// Remove the node from the account directory.
std::uint64_t const hint (signerList->getFieldU64 (sfOwnerNode));
auto const hint = (*signers)[sfOwnerNode];
TER const result = dirDelete(ctx_.view (), false, hint,
getOwnerDirIndex (account_), index, false, (hint == 0));
TER const result = dirDelete(ctx_.view(), false, hint,
ownerDirKeylet.key, signerListKeylet.key, false, (hint == 0));
if (result == tesSUCCESS)
adjustOwnerCount(view(),
view().peek(keylet::account(account_)),
removeFromOwnerCount);
view().peek(accountKeylet), removeFromOwnerCount);
ctx_.view ().erase (signerList);
ctx_.view().erase (signers);
return result;
}
// VFALCO NOTE This name is misleading, the signers
// are not written to the ledger they are
// added to the SLE.
void
SetSignerList::writeSignersToLedger (SLE::pointer ledgerEntry)
SetSignerList::writeSignersToSLE (SLE::pointer const& ledgerEntry) const
{
// Assign the quorum.
ledgerEntry->setFieldU32 (sfSignerQuorum, quorum_);
@@ -333,7 +327,9 @@ SetSignerList::writeSignersToLedger (SLE::pointer ledgerEntry)
ledgerEntry->setFieldArray (sfSignerEntries, toLedger);
}
std::size_t
// The return type is signed so it is compatible with the 3rd argument
// of adjustOwnerCount() (which must be signed).
int
SetSignerList::ownerCountDelta (std::size_t entryCount)
{
// We always compute the full change in OwnerCount, taking into account:
@@ -347,7 +343,12 @@ SetSignerList::ownerCountDelta (std::size_t entryCount)
// o And each signer in the list costs 1 more OwnerCount unit.
// So, at a minimum, adding a SignerList with 1 entry costs 3 OwnerCount
// units. A SignerList with 8 entries would cost 10 OwnerCount units.
return 2 + entryCount;
//
// The static_cast should always be safe since entryCount should always
// be in the range from 1 to 8. We've got a lot of room to grow.
assert (entryCount >= STTx::minMultiSigners);
assert (entryCount <= STTx::maxMultiSigners);
return 2 + static_cast<int>(entryCount);
}
} // namespace ripple

View File

@@ -76,12 +76,14 @@ private:
AccountID const& account,
beast::Journal j);
TER replaceSignerList (uint256 const& index);
TER destroySignerList (uint256 const& index);
TER replaceSignerList ();
TER destroySignerList ();
void writeSignersToLedger (SLE::pointer ledgerEntry);
TER removeSignersFromLedger (Keylet const& accountKeylet,
Keylet const& ownerDirKeylet, Keylet const& signerListKeylet);
void writeSignersToSLE (SLE::pointer const& ledgerEntry) const;
static std::size_t ownerCountDelta (std::size_t entryCount);
static int ownerCountDelta (std::size_t entryCount);
};
} // ripple

View File

@@ -185,7 +185,7 @@ enum TER
tecNO_LINE_REDUNDANT = 127,
tecPATH_DRY = 128,
tecUNFUNDED = 129, // Deprecated, old ambiguous unfunded.
tecMASTER_DISABLED = 130,
tecNO_ALTERNATIVE_KEY = 130,
tecNO_REGULAR_KEY = 131,
tecOWNERS = 132,
tecNO_ISSUER = 133,

View File

@@ -48,7 +48,7 @@ bool transResultInfo (TER code, std::string& token, std::string& text)
{ tecNO_LINE_REDUNDANT, "tecNO_LINE_REDUNDANT", "Can't set non-existent line to default." },
{ tecPATH_DRY, "tecPATH_DRY", "Path could not send partial amount." },
{ tecPATH_PARTIAL, "tecPATH_PARTIAL", "Path could not send full amount." },
{ tecMASTER_DISABLED, "tecMASTER_DISABLED", "Master key is disabled." },
{ tecNO_ALTERNATIVE_KEY, "tecNO_ALTERNATIVE_KEY", "The operation would remove the last way to sign a transaction."},
{ tecNO_REGULAR_KEY, "tecNO_REGULAR_KEY", "Regular key is not set." },
{ tecUNFUNDED, "tecUNFUNDED", "One of _ADD, _OFFER, or _SEND. Deprecated." },

View File

@@ -332,7 +332,7 @@ public:
env.require(nflags("alice", asfDisableMaster));
env(fset("alice", asfDisableMaster), sig("alice"));
env.require(flags("alice", asfDisableMaster));
env(regkey("alice", disabled), ter(tecMASTER_DISABLED));
env(regkey("alice", disabled), ter(tecNO_ALTERNATIVE_KEY));
env(noop("alice"));
env(noop("alice"), sig("alice"), ter(tefMASTER_DISABLED));
env(noop("alice"), sig("eric"));