Simple multisigning (RIPD-182):

With this changeset two-level multisigning is removed from the
codebase and replaced with single-level multisigning.

Additionally, SignerLists in the ledger are prepared for the
possibility of multiple SignerLists per account.  This was done
by adding a defaulted 32-bit SignerListID to each SignerList.
The SignerListIndex calculation incorporates the SignerListID.

There are three known missing elements:

 1. Multisigned transactions should require higher fees than
    regular (single-signed) transaction.  That's not yet
    implemented.

 2. It should be possible to disable the master key on an account
    if that account is multisign enabled (has a signer list).
    That's not yet implemented.

 3. Documentation about multisigning needs to be improved.

Multisigning is still compiled out of the code base.  To enable
multisigning for a stand-alone rippled, change the
RIPPLE_ENABLE_MULTI_SIGN macro (in BeastConfig.h) to "1" and
rebuild.

This commit also addresses:
 o RIPD-912: Remove multisign APIs from STObject, and
 o RIPD-944: Replace common_transactor with jtx at call sites.

Builds/VisualStudio2013/RippleD.vcxproj

@@ -1847,12 +1847,6 @@
     </ClInclude>
     <ClInclude Include="..\..\src\ripple\app\tx\LocalTxs.h">
     </ClInclude>
-    <ClCompile Include="..\..\src\ripple\app\tx\tests\common_transactor.cpp">
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
-    </ClCompile>
-    <ClInclude Include="..\..\src\ripple\app\tx\tests\common_transactor.h">
-    </ClInclude>
     <ClCompile Include="..\..\src\ripple\app\tx\tests\DeliverMin.test.cpp">
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>

Builds/VisualStudio2013/RippleD.vcxproj.filters

@@ -2592,12 +2592,6 @@
     <ClInclude Include="..\..\src\ripple\app\tx\LocalTxs.h">
       <Filter>ripple\app\tx</Filter>
     </ClInclude>
-    <ClCompile Include="..\..\src\ripple\app\tx\tests\common_transactor.cpp">
-      <Filter>ripple\app\tx\tests</Filter>
-    </ClCompile>
-    <ClInclude Include="..\..\src\ripple\app\tx\tests\common_transactor.h">
-      <Filter>ripple\app\tx\tests</Filter>
-    </ClInclude>
     <ClCompile Include="..\..\src\ripple\app\tx\tests\DeliverMin.test.cpp">
       <Filter>ripple\app\tx\tests</Filter>
     </ClCompile>

src/ripple/app/tx/README.md

@@ -97,7 +97,7 @@ issue a transaction that consumes that ticket.
 Any account can have one SignerList attached to it.  A SignerList contains the
 following elements:
 
- - A list of from 2 to a protocol-defined maximum of 8 signers.  Each signer in the array consists of:
+ - A list of from 1 to a protocol-defined maximum of 8 signers.  Each signer in the array consists of:
    - The signer's 160-bit account ID and
    - The signer's 16-bit weight (used to calculate whether a quorum is met).
  - And, for the entire list, a single 32-bit quorum value.

src/ripple/app/tx/impl/SetSignerList.cpp

@@ -32,6 +32,11 @@
 
 namespace ripple {
 
+// We're prepared for there to be multiple signer lists in the future,
+// but we don't need them yet.  So for the time being we're manually
+// setting the sfSignerListID to zero in all cases.
+static std::uint32_t const defaultSignerListID_ = 0;
+
 std::tuple<TER, std::uint32_t,
     std::vector<SignerEntries::SignerEntry>,
         SetSignerList::Operation>
@@ -47,17 +52,15 @@ SetSignerList::determineOperation(STTx const& tx,
     bool const hasSignerEntries(tx.isFieldPresent(sfSignerEntries));
     if (quorum && hasSignerEntries)
     {
-        SignerEntries::Decoded signers(
+        auto signers = SignerEntries::deserialize(tx, j, "transaction");
-            SignerEntries::deserialize(tx, j, "transaction"));
 
-        if (signers.ter != tesSUCCESS)
+        if (signers.second != tesSUCCESS)
-            return std::make_tuple(signers.ter,
+            return std::make_tuple(signers.second, quorum, sign, op);
-                quorum, sign, op);
 
-        std::sort(signers.vec.begin(), signers.vec.end());
+        std::sort(signers.first.begin(), signers.first.end());
 
         // Save deserialized list for later.
-        sign = std::move(signers.vec);
+        sign = std::move(signers.first);
         op = set;
     }
     else if ((quorum == 0) && !hasSignerEntries)
@@ -154,11 +157,10 @@ SetSignerList::validateQuorumAndSignerEntries (
     // Reject if there are too many or too few entries in the list.
     {
         std::size_t const signerCount = signers.size ();
-        if ((signerCount < SignerEntries::minEntries)
+        if ((signerCount < STTx::minMultiSigners)
-            || (signerCount > SignerEntries::maxEntries))
+            || (signerCount > STTx::maxMultiSigners))
         {
-            if (j.trace) j.trace <<
+            JLOG(j.trace) << "Too many or too few signers in signer list.";
-                "Too many or too few signers in signer list.";
             return temMALFORMED;
         }
     }
@@ -168,8 +170,7 @@ SetSignerList::validateQuorumAndSignerEntries (
     if (std::adjacent_find (
         signers.begin (), signers.end ()) != signers.end ())
     {
-        if (j.trace) j.trace <<
+        JLOG(j.trace) << "Duplicate signers in signer list";
-            "Duplicate signers in signer list";
         return temBAD_SIGNER;
     }
 
@@ -178,12 +179,18 @@ SetSignerList::validateQuorumAndSignerEntries (
     std::uint64_t allSignersWeight (0);
     for (auto const& signer : signers)
     {
+        std::uint32_t const weight = signer.weight;
+        if (weight <= 0)
+        {
+            JLOG(j.trace) << "Every signer must have a positive weight.";
+            return temBAD_WEIGHT;
+        }
+
         allSignersWeight += signer.weight;
 
         if (signer.account == account)
         {
-            if (j.trace) j.trace <<
+            JLOG(j.trace) << "A signer may not self reference account.";
-                "A signer may not self reference account.";
             return temBAD_SIGNER;
         }
 
@@ -192,8 +199,7 @@ SetSignerList::validateQuorumAndSignerEntries (
     }
     if ((quorum <= 0) || (allSignersWeight < quorum))
     {
-        if (j.trace) j.trace <<
+        JLOG(j.trace) << "Quorum is unreachable";
-            "Quorum is unreachable";
         return temBAD_QUORUM;
     }
     return tesSUCCESS;
@@ -241,9 +247,8 @@ SetSignerList::replaceSignerList (uint256 const& index)
     TER result = dirAdd(ctx_.view (),
         hint, getOwnerDirIndex (account_), index, describer);
 
-    if (j_.trace) j_.trace <<
+    JLOG(j_.trace) << "Create signer list for account " <<
-        "Create signer list for account " <<
+        toBase58(account_) << ": " << transHuman (result);
-        account_ << ": " << transHuman (result);
 
     if (result != tesSUCCESS)
         return result;
@@ -270,7 +275,7 @@ SetSignerList::destroySignerList (uint256 const& index)
 
     // We have to examine the current SignerList so we know how much to
     // reduce the OwnerCount.
-    std::uint32_t removeFromOwnerCount = 0;
+    std::int32_t removeFromOwnerCount = 0;
     auto const k = keylet::signers(account_);
     SLE::pointer accountSignersList =
         view().peek (k);
@@ -278,7 +283,7 @@ SetSignerList::destroySignerList (uint256 const& index)
     {
         STArray const& actualList =
             accountSignersList->getFieldArray (sfSignerEntries);
-        removeFromOwnerCount = ownerCountDelta (actualList.size ());
+        removeFromOwnerCount = ownerCountDelta (actualList.size ()) * -1;
     }
 
     // Remove the node from the account directory.
@@ -306,6 +311,9 @@ SetSignerList::writeSignersToLedger (SLE::pointer ledgerEntry)
     // Assign the quorum.
     ledgerEntry->setFieldU32 (sfSignerQuorum, quorum_);
 
+    // For now, assign the default SignerListID.
+    ledgerEntry->setFieldU32 (sfSignerListID, defaultSignerListID_);
+
     // Create the SignerListArray one SignerEntry at a time.
     STArray toLedger (signers_.size ());
     for (auto const& entry : signers_)
@@ -329,23 +337,12 @@ SetSignerList::ownerCountDelta (std::size_t entryCount)
     //  o Accounting for the number of entries in the list.
     // We can get away with that because lists are not adjusted incrementally;
     // we add or remove an entire list.
-
-    // The wiki (https://wiki.ripple.com/Multisign#Fees_2) currently says
-    // (December 2014) the reserve should be
-    //   Reserve * (N + 1) / 2
-    // That's not making sense to me right now, since I'm working in
-    // integral OwnerCount units.  If, say, N is 4 I don't know how to return
-    // 4.5 units as an integer.
     //
-    // So, just to get started, I'm saying that:
+    // The rule is:
     //  o Simply having a SignerList costs 2 OwnerCount units.
     //  o And each signer in the list costs 1 more OwnerCount unit.
-    // So, at a minimum, adding a SignerList with 2 entries costs 4 OwnerCount
+    // So, at a minimum, adding a SignerList with 1 entry costs 3 OwnerCount
     // units.  A SignerList with 8 entries would cost 10 OwnerCount units.
-    //
-    // It's worth noting that once this reserve policy has gotten into the
-    // wild it will be very difficult to change.  So think hard about what
-    // we want for the long term.
    return 2 + entryCount;
 }
 

src/ripple/app/tx/impl/SignerEntries.cpp

@@ -26,22 +26,23 @@
 
 namespace ripple {
 
-SignerEntries::Decoded
+std::pair<std::vector<SignerEntries::SignerEntry>, TER>
 SignerEntries::deserialize (
     STObject const& obj, beast::Journal journal, std::string const& annotation)
 {
-    Decoded s;
+    std::pair<std::vector<SignerEntry>, TER> s;
-    auto& accountVec (s.vec);
-    accountVec.reserve (maxEntries);
 
     if (!obj.isFieldPresent (sfSignerEntries))
     {
         if (journal.trace) journal.trace <<
             "Malformed " << annotation << ": Need signer entry array.";
-        s.ter = temMALFORMED;
+        s.second = temMALFORMED;
         return s;
     }
 
+    auto& accountVec = s.first;
+    accountVec.reserve (STTx::maxMultiSigners);
+
     STArray const& sEntries (obj.getFieldArray (sfSignerEntries));
     for (STObject const& sEntry : sEntries)
     {
@@ -50,7 +51,7 @@ SignerEntries::deserialize (
         {
             journal.trace <<
                 "Malformed " << annotation << ": Expected SignerEntry.";
-            s.ter = temMALFORMED;
+            s.second = temMALFORMED;
             return s;
         }
 
@@ -60,7 +61,7 @@ SignerEntries::deserialize (
         accountVec.emplace_back (account, weight);
     }
 
-    s.ter = tesSUCCESS;
+    s.second = tesSUCCESS;
     return s;
 }
 

src/ripple/app/tx/impl/SignerEntries.h

@@ -56,20 +56,13 @@ public:
         }
     };
 
-    struct Decoded
-    {
-        std::vector<SignerEntry> vec;
-        TER ter = temMALFORMED;
-    };
-
     // Deserialize a SignerEntries array from the network or from the ledger.
-    static Decoded deserialize (
+    static
+    std::pair<std::vector<SignerEntry>, TER>
+    deserialize (
         STObject const& obj,
         beast::Journal journal,
         std::string const& annotation);
-
-    static std::size_t const minEntries = 2;
-    static std::size_t const maxEntries = STTx::maxMultiSigners;
 };
 
 } // ripple

src/ripple/app/tx/impl/Transactor.cpp

@@ -107,7 +107,7 @@ TER Transactor::payFee ()
     // Only check fee is sufficient when the ledger is open.
     if (view().open() && saPaid < mFeeDue)
     {
-        j_.trace << "Insufficient fee paid: " <<
+        JLOG(j_.trace) << "Insufficient fee paid: " <<
             saPaid.getText () << "/" << mFeeDue.getText ();
 
         return telINSUF_FEE_P;
@@ -124,7 +124,7 @@ TER Transactor::payFee ()
 
     if (mSourceBalance < saPaid)
     {
-        j_.trace << "Insufficient balance:" <<
+        JLOG(j_.trace) << "Insufficient balance:" <<
             " balance=" << mSourceBalance.getText () <<
             " paid=" << saPaid.getText ();
 
@@ -162,7 +162,7 @@ TER Transactor::checkSeq ()
     {
         if (a_seq < t_seq)
         {
-            j_.trace <<
+            JLOG(j_.trace) <<
                 "applyTransaction: has future sequence number " <<
                 "a_seq=" << a_seq << " t_seq=" << t_seq;
             return terPRE_SEQ;
@@ -171,7 +171,7 @@ TER Transactor::checkSeq ()
         if (view().txExists(tx().getTransactionID ()))
             return tefALREADY;
 
-        j_.trace << "applyTransaction: has past sequence number " <<
+        JLOG(j_.trace) << "applyTransaction: has past sequence number " <<
             "a_seq=" << a_seq << " t_seq=" << t_seq;
         return tefPAST_SEQ;
     }
@@ -271,37 +271,33 @@ TER Transactor::checkSign ()
 
 TER Transactor::checkSingleSign ()
 {
-    // VFALCO NOTE This is needlessly calculating the
+    auto const sle = view().peek(keylet::account(account_));
-    //             AccountID multiple times.
+    if (! sle)
-
+        return tefFAILURE;  // We really expected to find the account.
-    // VFALCO What if sle is nullptr?
-    auto const sle = view().peek(
-        keylet::account(account_));
 
     // Consistency: Check signature
     // Verify the transaction's signing public key is authorized for signing.
-    if (calcAccountID(mSigningPubKey) == account_)
+    AccountID const idFromPubKey = calcAccountID(mSigningPubKey);
+    if (idFromPubKey == account_)
     {
         // Authorized to continue.
         mSigMaster = true;
         if (sle->isFlag(lsfDisableMaster))
             return tefMASTER_DISABLED;
     }
-    else if (mHasAuthKey &&
+    else if (mHasAuthKey && (idFromPubKey == sle->getAccountID (sfRegularKey)))
-        (calcAccountID(mSigningPubKey) ==
-            sle->getAccountID (sfRegularKey)))
     {
         // Authorized to continue.
     }
     else if (mHasAuthKey)
     {
-        j_.trace <<
+        JLOG(j_.trace) <<
             "applyTransaction: Delay: Not authorized to use account.";
         return tefBAD_AUTH;
     }
     else
     {
-        j_.trace <<
+        JLOG(j_.trace) <<
             "applyTransaction: Invalid: Not authorized to use account.";
         return tefBAD_AUTH_MASTER;
     }
@@ -309,101 +305,60 @@ TER Transactor::checkSingleSign ()
     return tesSUCCESS;
 }
 
-namespace TransactorDetail
+TER Transactor::checkMultiSign ()
 {
-
+    // Get mTxnAccountID's SignerList and Quorum.
-struct GetSignerListResult
+    std::shared_ptr<STLedgerEntry const> sleAccountSigners =
-{
+        view().read (keylet::signers(account_));
-    TER ter = tefFAILURE;
-    std::uint32_t quorum = std::numeric_limits <std::uint32_t>::max ();
-    std::vector<SignerEntries::SignerEntry> signerEntries;
-};
-
-// We need the SignerList for every SigningFor while multi-signing.
-static
-GetSignerListResult
-getSignerList (AccountID signingForAcctID,
-    ReadView const& view, beast::Journal journal)
-{
-    GetSignerListResult ret;
-
-    auto const k = keylet::signers(signingForAcctID);
-    auto const accountSignersList =
-        view.read (k);
-
     // If the signer list doesn't exist the account is not multi-signing.
-    if (!accountSignersList)
+    if (!sleAccountSigners)
     {
-        journal.trace <<
+        JLOG(j_.trace) <<
             "applyTransaction: Invalid: Not a multi-signing account.";
-        ret.ter = tefNOT_MULTI_SIGNING;
+        return tefNOT_MULTI_SIGNING;
-        return ret;
     }
-    ret.quorum = accountSignersList->getFieldU32 (sfSignerQuorum);
 
-    SignerEntries::Decoded signersOnAccountDecode =
+    // We have plans to support multiple SignerLists in the future.  The
-        SignerEntries::deserialize (*accountSignersList, journal, "ledger");
+    // presence and defaulted value of the SignerListID field will enable that.
-    ret.ter = signersOnAccountDecode.ter;
+    assert (sleAccountSigners->isFieldPresent (sfSignerListID));
+    assert (sleAccountSigners->getFieldU32 (sfSignerListID) == 0);
 
-    if (signersOnAccountDecode.ter == tesSUCCESS)
+    auto accountSigners =
-    {
+        SignerEntries::deserialize (*sleAccountSigners, j_, "ledger");
-        ret.signerEntries = std::move (signersOnAccountDecode.vec);
+    if (accountSigners.second != tesSUCCESS)
-    }
+        return accountSigners.second;
-    return ret;
-}
 
-struct CheckSigningAccountsResult
+    // Get the array of transaction signers.
-{
+    STArray const& txSigners (tx().getFieldArray (sfSigners));
-    TER ter = tefFAILURE;
-    std::uint32_t weightSum = 0;
-};
 
-// Verify that every SigningAccount is a valid SignerEntry.  Sum signer weights.
+    // Walk the accountSigners performing a variety of checks and see if
-CheckSigningAccountsResult
+    // the quorum is met.
-checkSigningAccounts (
-    std::vector<SignerEntries::SignerEntry> signerEntries,
-    STArray const& signingAccounts,
-    ApplyContext& ctx,
-    beast::Journal journal)
-{
-    CheckSigningAccountsResult ret;
 
-    // Both the signerEntries and signingAccounts are sorted by account.  So
+    // Both the multiSigners and accountSigners are sorted by account.  So
-    // matching signerEntries to signingAccounts should be a simple
+    // matching multi-signers to account signers should be a simple
     // linear walk.  *All* signers must be valid or the transaction fails.
     std::uint32_t weightSum = 0;
-    auto signerEntriesItr = signerEntries.begin ();
+    auto iter = accountSigners.first.begin ();
-    for (auto const& signingAccount : signingAccounts)
+    for (auto const& txSigner : txSigners)
     {
-        auto const signingAcctID =
+        AccountID const txSignerAcctID = txSigner.getAccountID (sfAccount);
-            signingAccount.getAccountID(sfAccount);
 
-        // Attempt to match the SignerEntry with a SigningAccount;
+        // Attempt to match the SignerEntry with a Signer;
-        while (signerEntriesItr->account < signingAcctID)
+        while (iter->account < txSignerAcctID)
         {
-            if (++signerEntriesItr == signerEntries.end ())
+            if (++iter == accountSigners.first.end ())
             {
-                journal.trace <<
+                JLOG(j_.trace) <<
                     "applyTransaction: Invalid SigningAccount.Account.";
-                ret.ter = tefBAD_SIGNATURE;
+                return tefBAD_SIGNATURE;
-                return ret;
             }
         }
-        if (signerEntriesItr->account != signingAcctID)
+        if (iter->account != txSignerAcctID)
         {
             // The SigningAccount is not in the SignerEntries.
-            journal.trace <<
+            JLOG(j_.trace) <<
                 "applyTransaction: Invalid SigningAccount.Account.";
-            ret.ter = tefBAD_SIGNATURE;
+            return tefBAD_SIGNATURE;
-            return ret;
-        }
-        if (signerEntriesItr->weight <= 0)
-        {
-            // The SigningAccount has a weight of zero and may not sign.
-            journal.trace <<
-                "applyTransaction: SigningAccount.Account needs weight > 0.";
-            ret.ter = tefBAD_SIGNATURE;
-            return ret;
         }
 
         // We found the SigningAccount in the list of valid signers.  Now we
@@ -411,7 +366,7 @@ checkSigningAccounts (
         // public key.
         AccountID const signingAcctIDFromPubKey =
             calcAccountID(RippleAddress::createAccountPublic (
-                signingAccount.getFieldVL (sfSigningPubKey)));
+                txSigner.getFieldVL (sfSigningPubKey)));
 
         // Verify that the signingAcctID and the signingAcctIDFromPubKey
         // belong together.  Here is are the rules:
@@ -438,24 +393,23 @@ checkSigningAccounts (
 
         // In any of these cases we need to know whether the account is in
         // the ledger.  Determine that now.
-        auto signersAccountRoot =
+        std::shared_ptr<STLedgerEntry const> sleTxSignerRoot =
-            ctx.view().read (keylet::account(signingAcctID));
+            view().read (keylet::account(txSignerAcctID));
 
-        if (signingAcctIDFromPubKey == signingAcctID)
+        if (signingAcctIDFromPubKey == txSignerAcctID)
         {
-            // Either Phantom or Master.  Phantom's automatically pass.
+            // Either Phantom or Master.  Phantoms automatically pass.
-            if (signersAccountRoot)
+            if (sleTxSignerRoot)
             {
                 // Master Key.  Account may not have asfDisableMaster set.
                 std::uint32_t const signerAccountFlags =
-                    signersAccountRoot->getFieldU32 (sfFlags);
+                    sleTxSignerRoot->getFieldU32 (sfFlags);
 
                 if (signerAccountFlags & lsfDisableMaster)
                 {
-                    journal.trace <<
+                    JLOG(j_.trace) <<
-                        "applyTransaction: MultiSignature lsfDisableMaster.";
+                        "applyTransaction: Signer:Account lsfDisableMaster.";
-                    ret.ter = tefMASTER_DISABLED;
+                    return tefMASTER_DISABLED;
-                    return ret;
                 }
             }
         }
@@ -463,158 +417,36 @@ checkSigningAccounts (
         {
             // May be a Regular Key.  Let's find out.
             // Public key must hash to the account's regular key.
-            if (!signersAccountRoot)
+            if (!sleTxSignerRoot)
             {
-                journal.trace <<
+                JLOG(j_.trace) <<
                     "applyTransaction: Non-phantom signer lacks account root.";
-                ret.ter = tefBAD_SIGNATURE;
+                return tefBAD_SIGNATURE;
-                return ret;
             }
 
-            if (!signersAccountRoot->isFieldPresent (sfRegularKey))
+            if (!sleTxSignerRoot->isFieldPresent (sfRegularKey))
             {
-                journal.trace <<
+                JLOG(j_.trace) <<
                     "applyTransaction: Account lacks RegularKey.";
-                ret.ter = tefBAD_SIGNATURE;
+                return tefBAD_SIGNATURE;
-                return ret;
             }
             if (signingAcctIDFromPubKey !=
-                signersAccountRoot->getAccountID (sfRegularKey))
+                sleTxSignerRoot->getAccountID (sfRegularKey))
             {
-                journal.trace <<
+                JLOG(j_.trace) <<
                     "applyTransaction: Account doesn't match RegularKey.";
-                ret.ter = tefBAD_SIGNATURE;
+                return tefBAD_SIGNATURE;
-                return ret;
             }
         }
         // The signer is legitimate.  Add their weight toward the quorum.
-        weightSum += signerEntriesItr->weight;
+        weightSum += iter->weight;
-    }
-    ret.weightSum = weightSum;
-    ret.ter = tesSUCCESS;
-    return ret;
-}
-
-} // namespace TransactorDetail
-
-TER Transactor::checkMultiSign ()
-{
-    // Get account_'s SignerList and Quorum.
-    using namespace TransactorDetail;
-    GetSignerListResult const outer =
-        getSignerList (account_, view(), j_);
-
-    if (outer.ter != tesSUCCESS)
-        return outer.ter;
-
-    // Get the actual array of transaction signers.
-    STArray const& multiSigners (tx().getFieldArray (sfMultiSigners));
-
-    // Walk the accountSigners performing a variety of checks and see if
-    // the quorum is met.
-
-    // Both the multiSigners and accountSigners are sorted by account.  So
-    // matching multi-signers to account signers should be a simple
-    // linear walk.  *All* signers must be valid or the transaction fails.
-    std::uint32_t weightSum = 0;
-    auto signerEntriesItr = outer.signerEntries.begin ();
-    for (auto const& signingFor : multiSigners)
-    {
-        auto const signingForID =
-            signingFor.getAccountID(sfAccount);
-
-        auto const& signingAccounts =
-            signingFor.getFieldArray (sfSigningAccounts);
-
-        // There are two possibilities:
-        //  o The signers are direct multi-signers for this account.
-        //  o The signers are signing for a multi-signer on this account.
-        // Handle those two cases separately.
-        if (signingForID == account_)
-        {
-            // The signers are direct multi-signers for this account.  Results
-            // from these signers directly effect the quorum.
-            CheckSigningAccountsResult const outerSigningAccountsResult =
-                checkSigningAccounts (
-                    outer.signerEntries, signingAccounts, ctx_, j_);
-
-            if (outerSigningAccountsResult.ter != tesSUCCESS)
-                return outerSigningAccountsResult.ter;
-
-            weightSum += outerSigningAccountsResult.weightSum;
-        }
-        else
-        {
-            // The signers are signing for a multi-signer on this account.
-            // Attempt to match the signingForID with a SignerEntry
-            while (signerEntriesItr->account < signingForID)
-            {
-                if (++signerEntriesItr == outer.signerEntries.end ())
-                {
-                    j_.trace <<
-                        "applyTransaction: Invalid SigningFor.Account.";
-                    return tefBAD_SIGNATURE;
-                }
-            }
-            if (signerEntriesItr->account != signingForID)
-            {
-                // The signingForID is not in the SignerEntries.
-                j_.trace <<
-                    "applyTransaction: Invalid SigningFor.Account.";
-                return tefBAD_SIGNATURE;
-            }
-            if (signerEntriesItr->weight <= 0)
-            {
-                // The SigningFor entry needs a weight greater than zero.
-                j_.trace <<
-                    "applyTransaction: SigningFor.Account needs weight > 0.";
-                return tefBAD_SIGNATURE;
-            }
-
-            // See if the signingForID has a SignerList.
-            GetSignerListResult const inner =
-                getSignerList (signingForID, view(), j_);
-
-            if (inner.ter != tesSUCCESS)
-                return inner.ter;
-
-            // Results from these signers indirectly effect the quorum.
-            CheckSigningAccountsResult const innerSigningAccountsResult =
-                checkSigningAccounts (
-                    inner.signerEntries, signingAccounts, ctx_, j_);
-
-            if (innerSigningAccountsResult.ter != tesSUCCESS)
-                return innerSigningAccountsResult.ter;
-
-            // There's a policy question here.  If the SigningAccounts are
-            // all valid but fail to reach this signingFor's Quorum do we:
-            //
-            //  1. Say the signature is valid but contributes 0 toward the
-            //     quorum?
-            //
-            //  2. Say that any SigningFor that doesn't meet quorum is an
-            //     invalid signature and fails?
-            //
-            // The choice is not obvious to me.  I'm picking 2 for now, since
-            // it's more restrictive.  We can switch to policy 1 later without
-            // causing transactions that would have worked before to fail.
-            // -- January 2015
-            if (innerSigningAccountsResult.weightSum < inner.quorum)
-            {
-                j_.trace <<
-                    "applyTransaction: Level-2 SigningFor did not make quorum.";
-                return tefBAD_QUORUM;
-            }
-            // This SigningFor met quorum.  Add its weight.
-            weightSum += signerEntriesItr->weight;
-        }
     }
 
     // Cannot perform transaction if quorum is not met.
-    if (weightSum < outer.quorum)
+    if (weightSum < sleAccountSigners->getFieldU32 (sfSignerQuorum))
     {
-        j_.trace <<
+        JLOG(j_.trace) <<
-            "applyTransaction: MultiSignature failed to meet quorum.";
+            "applyTransaction: Signers failed to meet quorum.";
         return tefBAD_QUORUM;
     }
 

src/ripple/app/tx/tests/common_transactor.cpp

@@ -1,435 +0,0 @@
-//------------------------------------------------------------------------------
-/*
-    This file is part of rippled: https://github.com/ripple/rippled
-    Copyright (c) 2012, 2013 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 <ripple/app/tx/tests/common_transactor.h>
-#include <ripple/app/main/Application.h>
-#include <ripple/app/ledger/LedgerConsensus.h>
-#include <ripple/app/ledger/LedgerTiming.h>
-#include <ripple/app/ledger/tests/common_ledger.h>
-#include <ripple/app/tx/apply.h>
-#include <ripple/basics/chrono.h>
-#include <ripple/protocol/TxFormats.h>
-#include <ripple/protocol/TxFlags.h>
-
-#if 0
-
-namespace ripple {
-namespace test {
-
-UserAccount::UserAccount (KeyType kType, std::string const& passphrase)
-{
-    RippleAddress const seed = RippleAddress::createSeedGeneric (passphrase);
-    master_ = generateKeysFromSeed (kType, seed);
-    acctID_ = calcAccountID(master_.publicKey);
-}
-
-void UserAccount::setRegKey (
-    TestLedger& ledger, KeyType kType, std::string const& passphrase)
-{
-    // Get information for the new regular key.
-    RippleAddress const seed = RippleAddress::createSeedGeneric(passphrase);
-    KeyPair regular = generateKeysFromSeed (kType, seed);
-
-    // Tell the ledger what we're up to.
-    STTx tx = getSetRegularKeyTx (*this,
-        calcAccountID(regular.publicKey));
-    singleSign (tx, *this);
-    ledger.applyGoodTransaction (tx);
-
-    // Remember what changed.
-    regular_ = regular;
-}
-
-void UserAccount::clrRegKey (TestLedger& ledger)
-{
-    // Tell the ledger what we're up to.
-    STTx tx = getClearRegularKeyTx (*this);
-    singleSign (tx, *this);
-    ledger.applyGoodTransaction (tx);
-
-    // Remember what changed.
-    RippleAddress temp;
-    regular_.secretKey = temp;
-    regular_.publicKey = temp;
-}
-
-void UserAccount::disableMaster (TestLedger& ledger, bool doDisable)
-{
-    STTx tx = getAccountSetTx (*this);
-    SField const& field = doDisable ? sfSetFlag : sfClearFlag;
-    tx.setFieldU32 (field, asfDisableMaster);
-    singleSign (tx, *this);
-    ledger.applyGoodTransaction (tx);
-}
-
-//------------------------------------------------------------------------------
-
-TestLedger::TestLedger (
-        std::uint64_t startAmountDrops,
-        UserAccount const& master,
-        beast::unit_test::suite& suite)
-: lastClosedLedger()
-, openLedger_()
-, suite_(suite)
-{
-    // To leverage createGenesisLedger from the Ledger tests, we must match
-    // its interface.
-    TestAccount const masterAcct {master.publicKey(), master.secretKey(), 0};
-    std::tie (lastClosedLedger, openLedger_) =
-        createGenesisLedger(startAmountDrops, masterAcct);
-}
-
-std::pair<TER, bool>
-TestLedger::applyTransaction (STTx const& tx, bool check)
-{
-    // Apply the transaction to the open ledger.
-    auto const r = apply(
-        *openLedger_, tx, check ? tapNONE : tapNO_CHECK_SIGN,
-            getConfig(), beast::Journal{});
-
-    // Close the open ledger to see if the transaction was real committed.
-    //
-    // In part we close the open ledger so we don't have to think about the
-    // time sequencing of transactions.  Every transaction applied by a
-    // call to this method gets applied individually.  So this transaction
-    // is guaranteed to be applied before the next one.
-    close_and_advance(openLedger_, lastClosedLedger);
-
-    // Check for the transaction in the closed ledger.
-    bool const foundTx =
-        lastClosedLedger->txExists(tx.getTransactionID());
-    suite_.expect (r.second == foundTx);
-
-    return {r.first, r.second && foundTx};
-}
-
-void TestLedger::applyGoodTransaction (STTx const& tx, bool check)
-{
-    auto ret = applyTransaction (tx, check);
-    suite_.expect (ret.first == tesSUCCESS);
-    suite_.expect (ret.second == true);
-}
-
-void TestLedger::applyBadTransaction (STTx const& tx, TER err, bool check)
-{
-    auto ret = applyTransaction (tx, check);
-    suite_.expect (ret.first == err);
-    suite_.expect (ret.second == false);
-}
-
-void TestLedger::applyTecTransaction (STTx const& tx, TER err, bool check)
-{
-    auto ret = applyTransaction (tx, check);
-    suite_.expect (ret.first == err);
-    suite_.expect (ret.second == true);
-}
-
-//------------------------------------------------------------------------------
-
-MultiSig::MultiSig(UserAccount const& signingFor,
-        UserAccount const& signer, STTx const& tx)
-: signingFor_(&signingFor)
-, signer_(&signer)
-, multiSig_()
-{
-    Serializer s = tx.getMultiSigningData (
-        calcAccountID(signingFor.acctPublicKey()),
-            calcAccountID(signer.acctPublicKey()));
-    multiSig_ = signer.secretKey().accountPrivateSign (s.getData());
-}
-
-//------------------------------------------------------------------------------
-
-void SignerList::injectInto (STTx& tx) const
-{
-    // Create the SignerListArray one STObject at a time.
-    STArray list (list_.size ());
-    for (auto const& entry : list_)
-    {
-        list.emplace_back(sfSignerEntry);
-        STObject& obj = list.back();
-        obj.reserve (2);
-        obj.setAccountID (sfAccount, entry.acct->getID ());
-        obj.setFieldU16 (sfSignerWeight, entry.weight);
-        obj.setTypeFromSField (sfSignerEntry);
-    }
-    // Insert the SignerEntries.
-    tx.setFieldArray (sfSignerEntries, list);
-}
-
-//------------------------------------------------------------------------------
-
-// Single-sign the passed transaction using acct.
-void singleSign (STTx& tx, UserAccount& acct)
-{
-    tx.setFieldVL (sfSigningPubKey, acct.publicKey().getAccountPublic ());
-    tx.sign (acct.secretKey ());
-}
-
-// Multi-sign the passed transaction using multiSigs.
-void multiSign (STTx& tx, std::vector<MultiSig>& multiSigs)
-{
-    // multiSigs must be sorted or the signature will fail.
-    std::sort(multiSigs.begin(), multiSigs.end());
-    insertMultiSigs(tx, multiSigs);
-}
-
-// Insert the multiSigs into tx without sorting.  Allows testing error cases.
-void insertMultiSigs (STTx& tx, std::vector<MultiSig> const& multiSigs)
-{
-    // Know when to change out SigningFor containers.
-    AccountID prevSigningForID;
-
-    // Create the MultiSigners array one STObject at a time.
-    STArray multiSigners;
-    boost::optional <STObject> signingFor;
-    for (auto const& entry : multiSigs)
-    {
-        if (entry.signingForAccount() != prevSigningForID)
-        {
-            if (signingFor != boost::none)
-                multiSigners.push_back (std::move(signingFor.get()));
-
-            // Construct the next SigningFor object and fill it in.
-            prevSigningForID = entry.signingForAccount();
-            signingFor.emplace (sfSigningFor);
-            signingFor->reserve (2);
-            signingFor->setAccountID (sfAccount, entry.signingForAccount());
-            signingFor->setFieldArray (sfSigningAccounts, STArray());
-        }
-        assert(signingFor);
-
-        // Construct this SigningAccount object and fill it in.
-        STArray& signingAccounts = signingFor->peekFieldArray(
-            sfSigningAccounts);
-        signingAccounts.emplace_back (sfSigningAccount);
-
-        STObject& signingAccount (signingAccounts.back());
-        signingAccount.reserve (3);
-        signingAccount.setAccountID (sfAccount, entry.signingAccount());
-        signingAccount.setFieldVL (sfMultiSignature, entry.multiSignature());
-        signingAccount.setFieldVL (sfSigningPubKey, entry.signingPubKey());
-    }
-    // Remember to put in the final SigningFor object.
-    if (signingFor)
-        multiSigners.push_back (std::move(signingFor.get()));
-
-    // Inject the multiSigners into tx.
-    tx.setFieldArray (sfMultiSigners, multiSigners);
-}
-
-//------------------------------------------------------------------------------
-
-// Return a transaction with an SOTemplate, sfTransactionType, sfAccount,
-// sfFee, sfFlags, and sfSequence.
-STTx getSeqTx (UserAccount& acct, TxType type)
-{
-    STTx tx (type); // Sets SOTemplate and sfTransactionType.
-    tx.setAccountID (sfAccount, acct.getID());
-    tx.setFieldAmount (sfFee, STAmount (10));
-    tx.setFieldU32 (sfFlags, tfUniversal);
-    tx.setFieldU32 (sfSequence, acct.consumeSeq());
-    return tx;
-}
-
-// Return an unsigned AccountSet transaction.
-STTx getAccountSetTx (UserAccount& acct)
-{
-    STTx tx = getSeqTx (acct, ttACCOUNT_SET);
-    return tx;
-}
-
-// Return an unsigned OfferCreate transaction.
-STTx getOfferCreateTx (UserAccount& acct,
-    STAmount const& takerGets, STAmount const& takerPays)
-{
-    STTx tx = getSeqTx (acct, ttOFFER_CREATE);
-    tx.setFieldAmount (sfTakerGets, takerGets);
-    tx.setFieldAmount (sfTakerPays, takerPays);
-    return tx;
-}
-
-// Return an unsigned OfferCancel transaction.
-STTx getOfferCancelTx (UserAccount& acct, std::uint32_t offerSeq)
-{
-    STTx tx = getSeqTx (acct, ttOFFER_CANCEL);
-    tx.setFieldU32 (sfOfferSequence, offerSeq);
-    return tx;
-}
-
-// Return an unsigned transaction good for making a payment.
-STTx getPaymentTx (
-    UserAccount& from, UserAccount const& to, std::uint64_t amountDrops)
-{
-    STTx tx = getSeqTx (from, ttPAYMENT);
-    tx.setAccountID (sfDestination, to.getID());
-    tx.setFieldAmount (sfAmount, amountDrops);
-    return tx;
-}
-
-STTx getPaymentTx (
-    UserAccount& from, UserAccount const& to, STAmount const& amount)
-{
-    STTx tx = getSeqTx (from, ttPAYMENT);
-    tx.setAccountID (sfDestination, to.getID());
-    tx.setFieldAmount (sfAmount, amount);
-    return tx;
-}
-
-// Return a transaction that sets a regular key
-STTx getSetRegularKeyTx (UserAccount& acct, AccountID const& regKey)
-{
-    STTx tx = getSeqTx (acct, ttREGULAR_KEY_SET);
-    tx.setAccountID (sfRegularKey,  regKey);
-    return tx;
-}
-
-// Return a transaction that clears a regular key
-STTx getClearRegularKeyTx (UserAccount& acct)
-{
-    STTx tx = getSeqTx (acct, ttREGULAR_KEY_SET);
-    return tx;
-}
-
-// Return a SignerListSet transaction.
-STTx getSignerListSetTx (
-    UserAccount& acct, SignerList const& signers, std::uint32_t quorum)
-{
-    STTx tx = getSeqTx (acct, ttSIGNER_LIST_SET);
-    tx.setFieldU32 (sfSignerQuorum, quorum);
-    if (! signers.empty ())
-    {
-        signers.injectInto (tx);
-    }
-    return tx;
-}
-
-// Return a transaction that creates an un-targeted ticket.
-STTx getCreateTicketTx (UserAccount& acct)
-{
-    STTx tx = getSeqTx (acct, ttTICKET_CREATE);
-    return tx;
-}
-
-// Return a transaction that creates a targeted ticket.
-STTx getCreateTicketTx (UserAccount& acct, UserAccount const& target)
-{
-    STTx tx = getSeqTx (acct, ttTICKET_CREATE);
-    tx.setAccountID (sfTarget, target.getID());
-    return tx;
-}
-
-// Return a transaction that cancels a ticket.
-STTx getCancelTicketTx (UserAccount& acct, uint256 const& ticketID)
-{
-    STTx tx = getSeqTx (acct, ttTICKET_CANCEL);
-    tx.setFieldH256 (sfTicketID, ticketID);
-    return tx;
-}
-
-// Return a trust set transaction.
-STTx getTrustSetTx (UserAccount& from, Issue const& issuer, int limit)
-{
-    STTx tx = getSeqTx (from, ttTRUST_SET);
-    STAmount const stLimit (issuer, limit);
-    tx.setFieldAmount (sfLimitAmount, stLimit);
-    return tx;
-}
-
-//------------------------------------------------------------------------------
-
-void payInDrops (TestLedger& ledger,
-    UserAccount& from, UserAccount const& to, std::uint64_t amountDrops)
-{
-    STTx tx = getPaymentTx (from, to, amountDrops);
-    singleSign (tx, from);
-    ledger.applyGoodTransaction (tx);
-}
-
-std::uint64_t getNativeBalance(TestLedger& ledger, UserAccount& acct)
-{
-    return ledger.lastClosedLedger->read(
-        keylet::account(acct.getID()))->getFieldAmount(
-            sfBalance).mantissa();
-}
-
-std::uint32_t getOwnerCount(TestLedger& ledger, UserAccount& acct)
-{
-    return ledger.lastClosedLedger->read(
-        keylet::account(acct.getID()))->getFieldU32(sfOwnerCount);
-}
-
-std::vector<RippleState::pointer> getRippleStates (
-    TestLedger& ledger, UserAccount const& acct, UserAccount const& peer)
-{
-    std::vector <RippleState::pointer> states;
-
-    forEachItem(*ledger.openLedger(), acct.getID(),
-        [&states, &acct, &peer](
-            std::shared_ptr<SLE const> const& sleCur)
-        {
-            // See whether this SLE is a lt_RIPPLE_STATE
-            if (!sleCur || sleCur->getType () != ltRIPPLE_STATE)
-                return;
-
-            // It's a lt_RIPPLE_STATE.  See if it's one we want to return.
-            RippleState::pointer const state (
-                RippleState::makeItem (acct.getID(), sleCur));
-
-            if ((state) && (state->getAccountIDPeer() == peer.getID()))
-                states.emplace_back (std::move (state));
-        });
-
-    return states;
-}
-
-std::vector <std::shared_ptr<SLE const>>
-getOffersOnAccount (TestLedger& ledger, UserAccount const& acct)
-{
-    std::vector <std::shared_ptr<SLE const>> offers;
-
-    forEachItem(*ledger.openLedger(), acct.getID(),
-        [&offers, &acct](
-            std::shared_ptr<SLE const> const& sleCur)
-        {
-            // If sleCur is an ltOFFER save it.
-            if (sleCur && sleCur->getType () == ltOFFER)
-                offers.emplace_back (sleCur);
-        });
-    return offers;
-}
-
-std::vector <std::shared_ptr<SLE const>>
-getTicketsOnAccount (TestLedger& ledger, UserAccount const& acct)
-{
-    std::vector <std::shared_ptr<SLE const>> offers;
-
-    forEachItem(*ledger.openLedger(), acct.getID(),
-        [&offers, &acct](
-            std::shared_ptr<SLE const> const& sleCur)
-        {
-            // If sleCur is an ltTICKET save it.
-            if (sleCur && sleCur->getType () == ltTICKET)
-                offers.emplace_back (sleCur);
-        });
-    return offers;
-}
-
-} // test
-} // ripple
-
-#endif

src/ripple/app/tx/tests/common_transactor.h

@@ -1,359 +0,0 @@
-//------------------------------------------------------------------------------
-/*
-    This file is part of rippled: https://github.com/ripple/rippled
-    Copyright (c) 2012, 2013 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.
-*/
-//==============================================================================
-
-#ifndef RIPPLE_APP_TRANSACTORS_TESTS_COMMON_TRANSACTOR_H_INCLUDED
-#define RIPPLE_APP_TRANSACTORS_TESTS_COMMON_TRANSACTOR_H_INCLUDED
-
-#include <ripple/app/ledger/Ledger.h>
-#include <ripple/app/paths/RippleState.h>
-#include <ripple/protocol/RippleAddress.h>
-#include <ripple/protocol/STAmount.h>
-#include <beast/unit_test/suite.h>
-#include <cstdint>
-
-namespace ripple {
-namespace test {
-
-// Forward declare TestLedger so UserAccount can take it as a parameter.
-class TestLedger;
-
-// This class makes it easier to write unit tests that involve user accounts.
-class UserAccount
-{
-private:
-    KeyPair master_;
-    AccountID acctID_;
-    KeyPair regular_;
-    bool useRegKey_ = false;
-    std::uint32_t sequence_ = 0;
-
-public:
-    UserAccount () = delete;
-    UserAccount (UserAccount const& rhs) = default;
-    UserAccount& operator= (UserAccount const& rhs) = default;
-
-    UserAccount (KeyType kType, std::string const& passphrase);
-
-    AccountID const& getID () const
-    {
-        return acctID_;
-    }
-
-    // Sets the regular key on the account, but does not disable master.
-    void setRegKey (
-        TestLedger& ledger, KeyType kType, std::string const& passphrase);
-
-    // Removes the regular key.
-    void clrRegKey (TestLedger&  ledger);
-
-    // Either disables or enables the master key.
-    void disableMaster (TestLedger& ledger, bool doDisable);
-
-    // Select to use either the regular (true) or master (false) key.
-    void useRegKey (bool useReg)
-    {
-        useRegKey_ = useReg;
-    }
-
-    std::uint32_t consumeSeq ()
-    {
-        return ++sequence_;
-    }
-
-    // If a transaction fails we have to back up the sequence number, since.
-    // the last sequence wasn't consumed.
-    void decrSeq ()
-    {
-        --sequence_;
-    }
-
-    RippleAddress const& acctPublicKey () const
-    {
-        return master_.publicKey;
-    }
-
-    RippleAddress const& publicKey () const
-    {
-        return useRegKey_ ? regular_.publicKey : master_.publicKey;
-    }
-
-    RippleAddress const& secretKey () const
-    {
-        return useRegKey_ ? regular_.secretKey : master_.secretKey;
-    }
-};
-
-// This class collects a bunch of the ledger shenanigans tests have to do.
-class TestLedger
-{
-public:
-    std::shared_ptr<Ledger const> lastClosedLedger;
-
-private:
-    Ledger::pointer openLedger_;
-    beast::unit_test::suite& suite_;
-
-public:
-    TestLedger () = delete;
-    TestLedger (const TestLedger& lhs) = delete;
-    TestLedger (
-        std::uint64_t startAmountDrops,
-        UserAccount const& master,
-        beast::unit_test::suite& suite);
-
-    // Return.first  : transaction's TEC
-    // Return.second : transaction successfully applied and in ledger.
-    std::pair<TER, bool> applyTransaction (STTx const& tx, bool check = true);
-
-    // Apply a transaction that we expect to succeed.
-    void applyGoodTransaction (STTx const& tx, bool check = true);
-
-    // Apply a transaction that we expect to fail.  Pass the expected error code.
-    void applyBadTransaction (STTx const& tx, TER err, bool check = true);
-
-    // apply a transaction that we expect to fail but charges a fee.  Pass the
-    // expected error code.
-    void applyTecTransaction (STTx const& tx, TER err, bool check = true);
-
-    // Return the current open ledger.
-    Ledger::pointer openLedger ()
-    {
-        return openLedger_;
-    }
-};
-
-// This class makes it easier to construct unit tests with SignerLists.
-//
-// Typically construct this class with an initializer list.  Like this:
-//
-// UserAccount alice;
-// UserAccount becky;
-// SignerList s {{alice, 5}, {becky, 2}};
-//
-// where '5' and '2' are the weights of the accounts in the SignerList.
-class SignerList
-{
-private:
-    // Our initializer list takes references that we turn into pointers.
-    struct InitListEntry
-    {
-        UserAccount& acct;
-        std::uint16_t weight;
-    };
-
-    struct SignerAndWeight
-    {
-        UserAccount* acct;
-        std::uint16_t weight;
-        SignerAndWeight(InitListEntry& entry)
-        : acct (&entry.acct)
-        , weight (entry.weight) { }
-    };
-    std::vector<SignerAndWeight> list_;
-
-public:
-    SignerList() = default;
-    SignerList(SignerList const& rhs) = default;
-    SignerList(SignerList&& rhs) // Visual Studio 2013 won't default move ctor
-    : list_(std::move(rhs.list_))
-    { }
-    SignerList(std::initializer_list<InitListEntry> args)
-    {
-        list_.reserve(args.size());
-        for (auto arg : args)
-        {
-            list_.emplace_back(arg);
-        }
-    }
-
-    SignerList& operator=(SignerList const& rhs) = default;
-    SignerList& operator=(SignerList&& rhs) // VS 2013 won't default move assign
-    {
-        list_ = std::move(rhs.list_);
-        return *this;
-    }
-
-    bool empty() const
-    {
-        return list_.empty ();
-    }
-
-    // Inject this SignerList into the passed transaction.
-    void injectInto (STTx& tx) const;
-};
-
-// This type makes it easier to write unit tests that use multi-signatures.
-//
-// The MultiSig type is intended to be short-lived (any user account it
-// references should out-live it).  It is also intended to be easily
-// moved in a std::vector, so it uses pointers not references.
-class MultiSig
-{
-    UserAccount const* signingFor_;
-    UserAccount const* signer_;
-    Blob multiSig_;
-
-public:
-    MultiSig() = delete;
-    MultiSig(MultiSig const& rhs) = default;
-    MultiSig(MultiSig&& rhs) // Visual Studio 2013 won't default move ctor
-    : signingFor_ (std::move(rhs.signingFor_))
-    , signer_ (std::move(rhs.signer_))
-    , multiSig_ (std::move(rhs.multiSig_))
-    { }
-
-    MultiSig& operator=(MultiSig const& rhs) = delete;
-    MultiSig& operator=(MultiSig&& rhs) // VS 2013 won't default move assign
-    {
-        signingFor_ = std::move(rhs.signingFor_);
-        signer_ = std::move(rhs.signer_);
-        multiSig_ = std::move(rhs.multiSig_);
-        return *this;
-    }
-
-    MultiSig(UserAccount const& signingFor,
-        UserAccount const& signer, STTx const& tx);
-
-    friend bool operator==(MultiSig const& lhs, MultiSig const& rhs)
-    {
-        return ((lhs.signingFor_->getID() == rhs.signingFor_->getID()) &&
-            (lhs.signer_->getID() == rhs.signer_->getID()));
-    }
-
-    friend bool operator<(MultiSig const& lhs, MultiSig const& rhs)
-    {
-        AccountID const& lhsSigingFor = lhs.signingFor_->getID();
-        AccountID const& rhsSigningFor = rhs.signingFor_->getID();
-        if (lhsSigingFor < rhsSigningFor)
-            return true;
-
-        if (lhsSigingFor > rhsSigningFor)
-            return false;
-
-        if (lhs.signer_->getID() < rhs.signer_->getID())
-            return true;
-
-        return false;
-    }
-
-    AccountID const& signingForAccount() const
-    {
-        return signingFor_->getID();
-    }
-
-    AccountID const& signingAccount() const
-    {
-        return signer_->getID();
-    }
-
-    Blob const& multiSignature() const
-    {
-        return multiSig_;
-    }
-
-    Blob const& signingPubKey() const
-    {
-        return signer_->publicKey().getAccountPublic();
-    }
-};
-
-//------------------------------------------------------------------------------
-
-// Single-sign the passed transaction using acct.
-void singleSign (STTx& tx, UserAccount& acct);
-
-// Multi-sign the passed transaction using multiSigs.
-void multiSign (STTx& tx, std::vector<MultiSig>& multiSigs);
-
-// Insert the multiSigs into tx without sorting.  Allows testing error cases.
-void insertMultiSigs (STTx& tx, std::vector<MultiSig> const& multiSigs);
-
-//------------------------------------------------------------------------------
-
-// Return a transaction with an SOTemplate, sfTransactionType, sfAccount,
-// sfFee, sfFlags, and sfSequence.
-STTx getSeqTx (UserAccount& acct, TxType type);
-
-// Get an AccountSet transaction.
-STTx getAccountSetTx (UserAccount& acct);
-
-// Return an unsigned OfferCreate transaction.
-STTx getOfferCreateTx (UserAccount& acct,
-    STAmount const& takerGets, STAmount const& takerPays);
-
-// Return an unsigned OfferCancel transaction.
-STTx getOfferCancelTx (UserAccount& acct, std::uint32_t offerSeq);
-
-// Return an unsigned transaction good for making a payment in XRP.
-STTx getPaymentTx (
-    UserAccount& from, UserAccount const& to, std::uint64_t amountDrops);
-
-// Return an unsigned transaction good for making a payment.
-STTx getPaymentTx (
-    UserAccount& from, UserAccount const& to, STAmount const& amount);
-
-// Return a transaction that sets a regular key.
-STTx getSetRegularKeyTx (UserAccount& acct, AccountID const& regKey);
-
-// Return a transaction that clears a regular key.
-STTx getClearRegularKeyTx (UserAccount& acct);
-
-// Return a SignerListSet transaction.  If the quorum is zero and signers
-// is empty, then any signer list is removed from the account.
-STTx getSignerListSetTx (
-    UserAccount& acct, SignerList const& signers, std::uint32_t quorum);
-
-// Return a transaction that creates an  un-targeted ticket.
-STTx getCreateTicketTx (UserAccount& acct);
-
-// Return a transaction that creates an  targeted ticket.
-STTx getCreateTicketTx (UserAccount& acct, UserAccount const& target);
-
-// Return a transaction that cancels a ticket.
-STTx getCancelTicketTx (UserAccount& acct, uint256 const& ticketID);
-
-// Return an unsigned trust set transaction.
-STTx getTrustSetTx (UserAccount& from, Issue const& issuer, int limit);
-
-//------------------------------------------------------------------------------
-
-// Complete the a simple Payment transaction in drops.  Expected to succeed.
-void payInDrops (TestLedger& ledger,
-    UserAccount& from, UserAccount const& to, std::uint64_t amountDrops);
-
-// Return the native balance on an account.
-std::uint64_t getNativeBalance(TestLedger& ledger, UserAccount& acct);
-
-// Return the owner count of an account.
-std::uint32_t getOwnerCount(TestLedger& ledger, UserAccount& acct);
-
-// Get all RippleStates between two accounts.
-std::vector<RippleState::pointer> getRippleStates (
-    TestLedger& ledger, UserAccount const& from, UserAccount const& peer);
-
-// Get all Offers on an account.
-std::vector <std::shared_ptr<SLE const>>
-getOffersOnAccount (TestLedger& ledger, UserAccount const& acct);
-
-// Get all Tickets on an account.
-std::vector <std::shared_ptr<SLE const>>
-getTicketsOnAccount (TestLedger& ledger, UserAccount const& acct);
-
-} // test
-} // ripple
-
-#endif // RIPPLE_APP_TRANSACTORS_TESTS_COMMON_TRANSACTOR_H_INCLUDED

src/ripple/protocol/SField.h

@@ -390,6 +390,7 @@ extern SF_U32 const sfClearFlag;
 extern SF_U32 const sfSignerQuorum;
 extern SF_U32 const sfCancelAfter;
 extern SF_U32 const sfFinishAfter;
+extern SF_U32 const sfSignerListID;
 
 // 64-bit integers
 extern SF_U64 const sfIndexNext;
@@ -462,9 +463,7 @@ extern SF_Blob const sfMemoData;
 extern SF_Blob const sfMemoFormat;
 
 // variable length (uncommon)
-extern SF_Blob const sfMultiSignature;
 extern SF_Blob const sfProof;
-
 // account
 extern SF_Account const sfAccount;
 extern SF_Account const sfOwner;
@@ -493,14 +492,13 @@ extern SField const sfNewFields;
 extern SField const sfTemplateEntry;
 extern SField const sfMemo;
 extern SField const sfSignerEntry;
-extern SField const sfSigningAccount;
+extern SField const sfSigner;
-extern SField const sfSigningFor;
 extern SField const sfMajority;
 
 // array of objects
 // ARRAY/1 is reserved for end of array
-extern SField const sfSigningAccounts;
+// extern SField const sfSigningAccounts;  // Never been used.
-extern SField const sfMultiSigners;
+extern SField const sfSigners;
 extern SField const sfSignerEntries;
 extern SField const sfTemplate;
 extern SField const sfNecessary;

src/ripple/protocol/STObject.h

@@ -423,20 +423,6 @@ public:
     uint256 getHash (std::uint32_t prefix) const;
     uint256 getSigningHash (std::uint32_t prefix) const;
 
-    // Break the multi-signing hash computation into 2 parts for optimization.
-    Serializer startMultiSigningData () const;
-    void finishMultiSigningData (
-        AccountID const& signingForID,
-        AccountID const& signingID, Serializer& s) const;
-
-    // VFALCO Get functions are usually simple observers but
-    //        this one performs an expensive construction.
-    //
-    // Get data to compute a complete multi-signature.
-    Serializer getMultiSigningData (
-        AccountID const& signingForID,
-        AccountID const& signingID) const;
-
     const STBase& peekAtIndex (int offset) const
     {
         return v_[offset].get();

src/ripple/protocol/STTx.h

@@ -45,6 +45,7 @@ public:
     using pointer = std::shared_ptr<STTx>;
     using ref     = const std::shared_ptr<STTx>&;
 
+    static std::size_t const minMultiSigners = 1;
     static std::size_t const maxMultiSigners = 8;
 
 public:

src/ripple/protocol/Sign.h

@@ -46,6 +46,31 @@ verify (STObject const& st,
         PublicKey const& pk,
             bool mustBeFullyCanonical);
 
+/** Return a Serializer suitable for computing a multisigning TxnSignature. */
+Serializer
+buildMultiSigningData (STObject const& obj, AccountID const& signingID);
+
+/** Break the multi-signing hash computation into 2 parts for optimization.
+
+    We can optimize verifying multiple multisignatures by splitting the
+    data building into two parts;
+     o A large part that is shared by all of the computations.
+     o A small part that is unique to each signer in the multisignature.
+
+    The following methods support that optimization:
+     1. startMultiSigningData provides the large part which can be shared.
+     2. finishMuiltiSigningData caps the passed in serializer with each
+        signer's unique data.
+*/
+Serializer
+startMultiSigningData (STObject const& obj);
+
+inline void
+finishMultiSigningData (AccountID const& signingID, Serializer& s)
+{
+    s.add160 (signingID);
+}
+
 } // ripple
 
 #endif

src/ripple/protocol/TER.h

@@ -83,6 +83,7 @@ enum TER
     temDISABLED,
     temBAD_SIGNER,
     temBAD_QUORUM,
+    temBAD_WEIGHT,
 
     // An intermediate result used internally, should never be returned.
     temUNCERTAIN,

src/ripple/protocol/impl/Indexes.cpp

@@ -181,9 +181,14 @@ getRippleStateIndex (AccountID const& a, Issue const& issue)
 uint256
 getSignerListIndex (AccountID const& account)
 {
+    // We are prepared for there to be multiple SignerLists in the future,
+    // but we don't have them yet.  In anticipation of multiple SignerLists
+    // We supply a 32-bit ID to locate the SignerList.  Until we actually
+    // *have* multiple signer lists, we can default that ID to zero.
     return sha512Half(
         std::uint16_t(spaceSignerList),
-        account);
+        account,
+        std::uint32_t (0));  // 0 == default SignerList ID.
 }
 
 //------------------------------------------------------------------------------

src/ripple/protocol/impl/InnerObjectFormats.cpp

@@ -29,15 +29,10 @@ InnerObjectFormats::InnerObjectFormats ()
         << SOElement (sfSignerWeight,         SOE_REQUIRED)
         ;
 
-    add (sfSigningFor.getJsonName ().c_str (), sfSigningFor.getCode ())
+    add (sfSigner.getJsonName ().c_str (), sfSigner.getCode ())
-        << SOElement (sfAccount,              SOE_REQUIRED)
-        << SOElement (sfSigningAccounts,      SOE_REQUIRED)
-        ;
-
-    add (sfSigningAccount.getJsonName ().c_str (), sfSigningAccount.getCode ())
         << SOElement (sfAccount,              SOE_REQUIRED)
         << SOElement (sfSigningPubKey,        SOE_REQUIRED)
-        << SOElement (sfMultiSignature,       SOE_REQUIRED)
+        << SOElement (sfTxnSignature,         SOE_REQUIRED)
         ;
 }
 

src/ripple/protocol/impl/LedgerFormats.cpp

@@ -122,12 +122,15 @@ LedgerFormats::LedgerFormats ()
             << SOElement (sfExpiration,          SOE_OPTIONAL)
             ;
 
-    // All three fields are SOE_REQUIRED because there is always a
+    // All fields are SOE_REQUIRED because there is always a
     // SignerEntries.  If there are no SignerEntries the node is deleted.
     add ("SignerList", ltSIGNER_LIST)
             << SOElement (sfOwnerNode,           SOE_REQUIRED)
             << SOElement (sfSignerQuorum,        SOE_REQUIRED)
             << SOElement (sfSignerEntries,       SOE_REQUIRED)
+            << SOElement (sfSignerListID,        SOE_REQUIRED)
+            << SOElement (sfPreviousTxnID,       SOE_REQUIRED)
+            << SOElement (sfPreviousTxnLgrSeq,   SOE_REQUIRED)
             ;
 }
 

src/ripple/protocol/impl/SField.cpp

@@ -123,6 +123,7 @@ SF_U32 const sfClearFlag           = make::one<SF_U32::type>(&sfClearFlag,
 SF_U32 const sfSignerQuorum        = make::one<SF_U32::type>(&sfSignerQuorum,        STI_UINT32, 35, "SignerQuorum");
 SF_U32 const sfCancelAfter         = make::one<SF_U32::type>(&sfCancelAfter,         STI_UINT32, 36, "CancelAfter");
 SF_U32 const sfFinishAfter         = make::one<SF_U32::type>(&sfFinishAfter,         STI_UINT32, 37, "FinishAfter");
+SF_U32 const sfSignerListID        = make::one<SF_U32::type>(&sfSignerListID,        STI_UINT32, 38, "SignerListID");
 
 // 64-bit integers
 SF_U64 const sfIndexNext     = make::one<SF_U64::type>(&sfIndexNext,     STI_UINT64, 1, "IndexNext");
@@ -195,7 +196,7 @@ SF_Blob const sfMemoData        = make::one<SF_Blob::type>(&sfMemoData,      STI
 SF_Blob const sfMemoFormat      = make::one<SF_Blob::type>(&sfMemoFormat,    STI_VL, 14, "MemoFormat");
 
 // variable length (uncommon)
-SF_Blob const sfMultiSignature = make::one<SF_Blob::type>(&sfMultiSignature, STI_VL, 16, "MultiSignature");
+//                                                                                   16 has not been used yet...
 SF_Blob const sfProof          = make::one<SF_Blob::type>(&sfProof,          STI_VL, 17, "Proof");
 
 // account
@@ -228,14 +229,14 @@ SField const sfMemo                = make::one(&sfMemo,                STI_OBJEC
 SField const sfSignerEntry         = make::one(&sfSignerEntry,         STI_OBJECT, 11, "SignerEntry");
 
 // inner object (uncommon)
-SField const sfSigningAccount      = make::one(&sfSigningAccount,      STI_OBJECT, 16, "SigningAccount");
+SField const sfSigner              = make::one(&sfSigner,              STI_OBJECT, 16, "Signer");
-SField const sfSigningFor          = make::one(&sfSigningFor,          STI_OBJECT, 17, "SigningFor");
+//                                                                                 17 has not been used yet...
 SField const sfMajority            = make::one(&sfMajority,            STI_OBJECT, 18, "Majority");
 
 // array of objects
 // ARRAY/1 is reserved for end of array
-SField const sfSigningAccounts = make::one(&sfSigningAccounts, STI_ARRAY, 2, "SigningAccounts");
+// SField const sfSigningAccounts = make::one(&sfSigningAccounts, STI_ARRAY, 2, "SigningAccounts"); // Never been used.
-SField const sfMultiSigners    = make::one(&sfMultiSigners,    STI_ARRAY, 3, "MultiSigners", SField::sMD_Default, SField::notSigning);
+SField const sfSigners         = make::one(&sfSigners,         STI_ARRAY, 3, "Signers", SField::sMD_Default, SField::notSigning);
 SField const sfSignerEntries   = make::one(&sfSignerEntries,   STI_ARRAY, 4, "SignerEntries");
 SField const sfTemplate        = make::one(&sfTemplate,        STI_ARRAY, 5, "Template");
 SField const sfNecessary       = make::one(&sfNecessary,       STI_ARRAY, 6, "Necessary");

src/ripple/protocol/impl/STObject.cpp

@@ -338,36 +338,6 @@ uint256 STObject::getSigningHash (std::uint32_t prefix) const
     return s.getSHA512Half ();
 }
 
-Serializer
-STObject::startMultiSigningData () const
-{
-    Serializer s;
-    s.add32 (HashPrefix::txMultiSign);
-    add (s, false);
-    return s;
-}
-
-// VFALCO This should not be a member,
-//        and the function shouldn't even exist
-void
-STObject::finishMultiSigningData (
-    AccountID const& signingForID,
-    AccountID const& signingID,
-    Serializer& s) const
-{
-    s.add160 (signingForID);
-    s.add160 (signingID);
-}
-
-Serializer
-STObject::getMultiSigningData (
-    AccountID const& signingForID, AccountID const& signingID) const
-{
-    Serializer s (startMultiSigningData ());
-    finishMultiSigningData (signingForID, signingID, s);
-    return s;
-}
-
 int STObject::getFieldIndex (SField const& field) const
 {
     if (mType != nullptr)

src/ripple/protocol/impl/STTx.cpp

@@ -22,6 +22,7 @@
 #include <ripple/protocol/HashPrefix.h>
 #include <ripple/protocol/JsonFields.h>
 #include <ripple/protocol/Protocol.h>
+#include <ripple/protocol/Sign.h>
 #include <ripple/protocol/STAccount.h>
 #include <ripple/protocol/STArray.h>
 #include <ripple/protocol/TxFlags.h>
@@ -189,8 +190,8 @@ bool STTx::checkSign(bool allowMultiSign) const
         if (allowMultiSign)
         {
             // Determine whether we're single- or multi-signing by looking
-            // at the SigningPubKey.  It it's empty we must be multi-signing.
+            // at the SigningPubKey.  It it's empty we must be
-            // Otherwise we're single-signing.
+            // multi-signing.  Otherwise we're single-signing.
             Blob const& signingPubKey = getFieldVL (sfSigningPubKey);
             sigGood = signingPubKey.empty () ?
                 checkMultiSign () : checkSingleSign ();
@@ -269,10 +270,10 @@ STTx::getMetaSQL (Serializer rawTxn,
 bool
 STTx::checkSingleSign () const
 {
-    // We don't allow both a non-empty sfSigningPubKey and an sfMultiSigners.
+    // We don't allow both a non-empty sfSigningPubKey and an sfSigners.
     // That would allow the transaction to be signed two ways.  So if both
     // fields are present the signature is invalid.
-    if (isFieldPresent (sfMultiSigners))
+    if (isFieldPresent (sfSigners))
         return false;
 
     bool ret = false;
@@ -301,22 +302,19 @@ STTx::checkMultiSign () const
 {
     // Make sure the MultiSigners are present.  Otherwise they are not
     // attempting multi-signing and we just have a bad SigningPubKey.
-    if (!isFieldPresent (sfMultiSigners))
+    if (!isFieldPresent (sfSigners))
         return false;
 
-    STArray const& multiSigners (getFieldArray (sfMultiSigners));
+    STArray const& signers {getFieldArray (sfSigners)};
 
     // There are well known bounds that the number of signers must be within.
-    {
+    if (signers.size() < minMultiSigners || signers.size() > maxMultiSigners)
-        std::size_t const multiSignerCount = multiSigners.size ();
-        if ((multiSignerCount < 1) || (multiSignerCount > maxMultiSigners))
         return false;
-    }
 
     // We can ease the computational load inside the loop a bit by
     // pre-constructing part of the data that we hash.  Fill a Serializer
     // with the stuff that stays constant from signature to signature.
-    Serializer const dataStart (startMultiSigningData ());
+    Serializer const dataStart {startMultiSigningData (*this)};
 
     // We also use the sfAccount field inside the loop.  Get it once.
     auto const txnAccountID = getAccountID (sfAccount);
@@ -326,139 +324,36 @@ STTx::checkMultiSign () const
         ? ECDSA::strict
         : ECDSA::not_strict;
 
-    /*
+    // Signers must be in sorted order by AccountID.
-        We need to detect (and reject) if a multi-signer is both signing
+    AccountID lastAccountID (beast::zero);
-        directly and using a SigningFor.  Here's an example:
 
+    for (auto const& signer : signers)
     {
-            ...
+        auto const accountID = signer.getAccountID (sfAccount);
-            "MultiSigners": [
-                {
-                    "SigningFor": {
-                        "Account": "<alice>",
-                        "SigningAccounts": [
-                            {
-                                "SigningAccount": {
-                                    // * becky says that becky signs for alice. *
-                                    "Account": "<becky>",
-            ...
-                    "SigningFor": {
-                        "Account": "<becky>",
-                        "SigningAccounts": [
-                            {
-                                "SigningAccount": {
-                                    // * cheri says that becky signs for alice. *
-                                    "Account": "<cheri>",
-            ...
-            "tx_json": {
-                "Account": "<alice>",
-                ...
-            }
-        }
 
-        Why is this way of signing a problem?  Alice has a signer list, and
+        // The account owner may not multisign for themselves.
-        Becky can show up in that list only once.  By design.  So if Becky
+        if (accountID == txnAccountID)
-        signs twice -- once directly and once indirectly -- we have three
-        options:
-        
-         1. We can add Becky's weight toward Alice's quorum twice, once for
-            each signature.  This seems both unexpected and counter to Alice's
-            intention.
-        
-         2. We could allow both signatures, but only add Becky's weight
-            toward Alice's quorum once.  This seems a bit better.  But it allows
-            our clients to ask rippled to do more work than necessary.  We
-            should also let the client know that only one of the signatures
-            was necessary.
-        
-         3. The only way to tell the client that they have done more work
-            than necessary (and that one of the signatures will be ignored) is
-            to declare the transaction malformed.  This behavior also aligns
-            well with rippled's behavior if Becky had signed directly twice:
-            the transaction would be marked as malformed.
-    */
-
-    // We use this std::set to detect this form of double-signing.
-    std::set<AccountID> firstLevelSigners;
-
-    // SigningFors must be in sorted order by AccountID.
-    AccountID lastSigningForID = zero;
-
-    // Every signature must verify or we reject the transaction.
-    for (auto const& signingFor : multiSigners)
-    {
-        auto const signingForID =
-            signingFor.getAccountID (sfAccount);
-
-        // SigningFors must be in order by account ID.  No duplicates allowed.
-        if (lastSigningForID >= signingForID)
-            return false;
-
-        // The next SigningFor must be greater than this one.
-        lastSigningForID = signingForID;
-
-        // If signingForID is *not* txnAccountID, then look for duplicates.
-        bool const directSigning = (signingForID == txnAccountID);
-        if (! directSigning)
-        {
-            if (! firstLevelSigners.insert (signingForID).second)
-                // This is a duplicate signer.  Fail.
-                return false;
-        }
-
-        STArray const& signingAccounts (
-            signingFor.getFieldArray (sfSigningAccounts));
-
-        // There are bounds that the number of signers must be within.
-        {
-            std::size_t const signingAccountsCount = signingAccounts.size ();
-            if ((signingAccountsCount < 1) ||
-                (signingAccountsCount > maxMultiSigners))
-            {
-                return false;
-            }
-        }
-
-        // SingingAccounts must be in sorted order by AccountID.
-        AccountID lastSigningAcctID = zero;
-
-        for (auto const& signingAcct : signingAccounts)
-        {
-            auto const signingAcctID =
-                signingAcct.getAccountID (sfAccount);
-
-            // None of the multi-signers may sign for themselves.
-            if (signingForID == signingAcctID)
             return false;
 
         // Accounts must be in order by account ID.  No duplicates allowed.
-            if (lastSigningAcctID >= signingAcctID)
+        if (lastAccountID >= accountID)
             return false;
 
         // The next signature must be greater than this one.
-            lastSigningAcctID = signingAcctID;
+        lastAccountID = accountID;
-
-            // If signingForID *is* txnAccountID, then look for duplicates.
-            if (directSigning)
-            {
-                if (! firstLevelSigners.insert (signingAcctID).second)
-                    // This is a duplicate signer.  Fail.
-                    return false;
-            }
 
         // Verify the signature.
         bool validSig = false;
         try
         {
             Serializer s = dataStart;
-                finishMultiSigningData (signingForID, signingAcctID, s);
+            finishMultiSigningData (accountID, s);
 
             RippleAddress const pubKey =
                 RippleAddress::createAccountPublic (
-                        signingAcct.getFieldVL (sfSigningPubKey));
+                    signer.getFieldVL (sfSigningPubKey));
 
-                Blob const signature =
+            Blob const signature = signer.getFieldVL (sfTxnSignature);
-                    signingAcct.getFieldVL (sfMultiSignature);
 
             validSig = pubKey.accountPublicVerify (
                 s.getData(), signature, fullyCanonical);
@@ -471,7 +366,6 @@ STTx::checkMultiSign () const
         if (!validSig)
             return false;
     }
-    }
 
     // All signatures verified.
     return true;

src/ripple/protocol/impl/Sign.cpp

@@ -51,4 +51,21 @@ verify (STObject const& st,
                 true);
 }
 
+Serializer
+buildMultiSigningData (STObject const& obj, AccountID const& signingID)
+{
+    Serializer s {startMultiSigningData (obj)};
+    finishMultiSigningData (signingID, s);
+    return s;
+}
+
+Serializer
+startMultiSigningData (STObject const& obj)
+{
+    Serializer s;
+    s.add32 (HashPrefix::txMultiSign);
+    obj.addWithoutSigningFields (s);
+    return s;
+}
+
 } // ripple

src/ripple/protocol/impl/TER.cpp

@@ -113,9 +113,10 @@ bool transResultInfo (TER code, std::string& token, std::string& text)
         { temBAD_SEND_XRP_PATHS,    "temBAD_SEND_XRP_PATHS",    "Malformed: Paths are not allowed for XRP to XRP."              },
         { temBAD_SEQUENCE,          "temBAD_SEQUENCE",          "Malformed: Sequence is not in the past."                       },
         { temBAD_SIGNATURE,         "temBAD_SIGNATURE",         "Malformed: Bad signature."                                     },
-        { temBAD_SIGNER,            "temBAD_SIGNER",            "Malformed: A signer may not duplicate account or other signers"},
+        { temBAD_SIGNER,            "temBAD_SIGNER",            "Malformed: No signer may duplicate account or other signers."  },
         { temBAD_SRC_ACCOUNT,       "temBAD_SRC_ACCOUNT",       "Malformed: Bad source account."                                },
         { temBAD_TRANSFER_RATE,     "temBAD_TRANSFER_RATE",     "Malformed: Transfer rate must be >= 1.0"                       },
+        { temBAD_WEIGHT,            "temBAD_WEIGHT",            "Malformed: Weight must be a positive value."                   },
         { temDST_IS_SRC,            "temDST_IS_SRC",            "Destination may not be source."                                },
         { temDST_NEEDED,            "temDST_NEEDED",            "Destination not specified."                                    },
         { temINVALID,               "temINVALID",               "The transaction is ill-formed."                                },

src/ripple/protocol/impl/TxFormats.cpp

@@ -132,7 +132,7 @@ void TxFormats::addCommonFields (Item& item)
         << SOElement(sfMemos,               SOE_OPTIONAL)
         << SOElement(sfSigningPubKey,       SOE_REQUIRED)
         << SOElement(sfTxnSignature,        SOE_OPTIONAL)
-        << SOElement(sfMultiSigners,        SOE_OPTIONAL) // submit_multisigned
+        << SOElement(sfSigners,             SOE_OPTIONAL) // submit_multisigned
         ;
 }
 

src/ripple/protocol/tests/STTx.test.cpp

@@ -18,6 +18,7 @@
 //==============================================================================
 
 #include <BeastConfig.h>
+#include <ripple/protocol/Sign.h>
 #include <ripple/protocol/STTx.h>
 #include <ripple/protocol/STParsedJSON.h>
 #include <ripple/protocol/types.h>
@@ -113,44 +114,34 @@ public:
             saSeed.createAccountPrivate(saGenerator, saSeed, 0);
 
         // Get the stream of the transaction for use in multi-signing.
-        Serializer s = txn.getMultiSigningData(
+        Serializer s = buildMultiSigningData (txn, saID);
-            calcAccountID(saPublicAcct), calcAccountID(saPublicAcct));
 
         Blob saMultiSignature =
             saPrivateAcct.accountPrivateSign (s.getData());
 
-        // The InnerObjectFormats say a SigningAccount is supposed to look
+        // The InnerObjectFormats say a Signer is supposed to look
         // like this:
-        // SigningAccount {
+        // Signer {
         //     Account: "...",
-        //     MultiSignature: "...",
+        //     TxnSignature: "...",
         //     PublicKey: "...""
         // }
-        // Make one well formed SigningAccount and several mal-formed ones.
+        // Make one well formed Signer and several mal-formed ones.  See
-        // See whether the serializer lets the good one through and catches
+        // whether the serializer lets the good one through and catches
         // the bad ones.
 
         // This lambda contains the bulk of the test code.
         auto testMalformedSigningAccount =
             [this, &txn, &signingForID]
-                (STObject const& signingAcct, bool expectPass)
+                (STObject const& signer, bool expectPass)
         {
             // Create SigningAccounts array.
-            STArray signingAccts (sfSigningAccounts, 1);
+            STArray signers (sfSigners, 1);
-            signingAccts.push_back (signingAcct);
+            signers.push_back (signer);
 
-            // Insert SigningAccounts array into SigningFor object.
+            // Insert signers into transaction.
-            STObject signingFor (sfSigningFor);
-            signingFor.setAccountID (sfAccount, signingForID);
-            signingFor.setFieldArray (sfSigningAccounts, signingAccts);
-
-            // Insert SigningFor into MultiSigners.
-            STArray multiSigners (sfMultiSigners, 1);
-            multiSigners.push_back (signingFor);
-
-            // Insert MultiSigners into transaction.
             STTx tempTxn (txn);
-            tempTxn.setFieldArray (sfMultiSigners, multiSigners);
+            tempTxn.setFieldArray (sfSigners, signers);
 
             Serializer rawTxn;
             tempTxn.add (rawTxn);
@@ -167,41 +158,41 @@ public:
             }
             expect (serialized == expectPass,
                 "Unexpected serialized = " + std::to_string (serialized) +
-                      ".  Object:\n" + signingAcct.getFullText () + "\n");
+                      ".  Object:\n" + signer.getFullText () + "\n");
         };
 
         {
-            // Test case 1.  Make a valid SigningAccount object.
+            // Test case 1.  Make a valid Signer object.
-            STObject soTest1 (sfSigningAccount);
+            STObject soTest1 (sfSigner);
             soTest1.setAccountID (sfAccount, saID);
             soTest1.setFieldVL (sfSigningPubKey,
                 txnPublicAcct.getAccountPublic ());
-            soTest1.setFieldVL (sfMultiSignature, saMultiSignature);
+            soTest1.setFieldVL (sfTxnSignature, saMultiSignature);
             testMalformedSigningAccount (soTest1, true);
         }
         {
             // Test case 2.  Omit sfSigningPubKey from SigningAccount.
-            STObject soTest2 (sfSigningAccount);
+            STObject soTest2 (sfSigner);
             soTest2.setAccountID (sfAccount, saID);
-            soTest2.setFieldVL (sfMultiSignature, saMultiSignature);
+            soTest2.setFieldVL (sfTxnSignature, saMultiSignature);
             testMalformedSigningAccount (soTest2, false);
         }
         {
             // Test case 3.  Extra sfAmount in SigningAccount.
-            STObject soTest3 (sfSigningAccount);
+            STObject soTest3 (sfSigner);
             soTest3.setAccountID (sfAccount, saID);
             soTest3.setFieldVL (sfSigningPubKey,
                 txnPublicAcct.getAccountPublic ());
-            soTest3.setFieldVL (sfMultiSignature, saMultiSignature);
+            soTest3.setFieldVL (sfTxnSignature, saMultiSignature);
             soTest3.setFieldAmount (sfAmount, STAmount (10000));
             testMalformedSigningAccount (soTest3, false);
         }
         {
             // Test case 4.  Right number of fields, but wrong ones.
-            STObject soTest4 (sfSigningAccount);
+            STObject soTest4 (sfSigner);
             soTest4.setFieldVL (sfSigningPubKey,
                 txnPublicAcct.getAccountPublic ());
-            soTest4.setFieldVL (sfMultiSignature, saMultiSignature);
+            soTest4.setFieldVL (sfTxnSignature, saMultiSignature);
             soTest4.setFieldAmount (sfAmount, STAmount (10000));
             testMalformedSigningAccount (soTest4, false);
         }

src/ripple/rpc/handlers/AccountInfo.cpp

@@ -71,23 +71,6 @@ Json::Value doAccountInfo (RPC::Context& context)
     if (sleAccepted)
     {
         RPC::injectSLE(jvAccepted, *sleAccepted);
-
-        // See if there's a SignerEntries for this account.
-        auto const signerList = ledger->read (keylet::signers(accountID));
-
-        if (signerList)
-        {
-            // Return multi-signing information if there are multi-signers.
-            static const Json::StaticString multiSignersName("multisigners");
-            jvAccepted[multiSignersName] = signerList->getJson (0);
-            Json::Value& multiSignerJson = jvAccepted[multiSignersName];
-
-            // Remove unwanted fields.
-            multiSignerJson.removeMember (sfFlags.getName ());
-            multiSignerJson.removeMember (sfLedgerEntryType.getName ());
-            multiSignerJson.removeMember (sfOwnerNode.getName ());
-            multiSignerJson.removeMember ("index");
-        }
         result[jss::account_data] = jvAccepted;
     }
     else

src/ripple/rpc/handlers/Handlers.h

@@ -68,6 +68,7 @@ Json::Value doSign                  (RPC::Context&);
 Json::Value doSignFor               (RPC::Context&);
 Json::Value doStop                  (RPC::Context&);
 Json::Value doSubmit                (RPC::Context&);
+Json::Value doSubmitMultiSigned     (RPC::Context&);
 Json::Value doSubscribe             (RPC::Context&);
 Json::Value doTransactionEntry      (RPC::Context&);
 Json::Value doTx                    (RPC::Context&);

src/ripple/rpc/impl/TransactionSign.cpp

@@ -27,9 +27,11 @@
 #include <ripple/json/json_reader.h>
 #include <ripple/json/json_writer.h>
 #include <ripple/net/RPCErr.h>
+#include <ripple/protocol/Sign.h>
 #include <ripple/protocol/ErrorCodes.h>
 #include <ripple/protocol/STParsedJSON.h>
 #include <ripple/protocol/TxFlags.h>
+#include <ripple/protocol/UintTypes.h>
 #include <ripple/rpc/impl/KeypairForSignature.h>
 #include <ripple/rpc/impl/LegacyPathFind.h>
 #include <ripple/rpc/impl/TransactionSign.h>
@@ -44,14 +46,12 @@ namespace detail {
 class SigningForParams
 {
 private:
-    AccountID const* const signingForAcctID_;
     AccountID const* const multiSigningAcctID_;
     RippleAddress* const multiSignPublicKey_;
     Blob* const multiSignature_;
 public:
     explicit SigningForParams ()
-    : signingForAcctID_ (nullptr)
+    : multiSigningAcctID_ (nullptr)
-    , multiSigningAcctID_ (nullptr)
     , multiSignPublicKey_ (nullptr)
     , multiSignature_ (nullptr)
     { }
@@ -59,12 +59,10 @@ public:
     SigningForParams (SigningForParams const& rhs) = delete;
 
     SigningForParams (
-        AccountID const& signingForAcctID,
         AccountID const& multiSigningAcctID,
         RippleAddress& multiSignPublicKey,
         Blob& multiSignature)
-    : signingForAcctID_ (&signingForAcctID)
+    : multiSigningAcctID_ (&multiSigningAcctID)
-    , multiSigningAcctID_ (&multiSigningAcctID)
     , multiSignPublicKey_ (&multiSignPublicKey)
     , multiSignature_ (&multiSignature)
     { }
@@ -72,7 +70,6 @@ public:
     bool isMultiSigning () const
     {
         return ((multiSigningAcctID_ != nullptr) &&
-                (signingForAcctID_ != nullptr) &&
                 (multiSignPublicKey_ != nullptr) &&
                 (multiSignature_ != nullptr));
     }
@@ -84,11 +81,6 @@ public:
     }
 
     // Don't call this method unless isMultiSigning() returns true.
-    AccountID const& getSigningFor ()
-    {
-        return *signingForAcctID_;
-    }
-
     AccountID const& getSigner ()
     {
         return *multiSigningAcctID_;
@@ -345,9 +337,8 @@ Json::Value checkFee (
     if (fee > limit)
     {
         std::stringstream ss;
-        ss <<
+        ss << "Fee of " << fee
-            "Fee of " << fee <<
+            << " exceeds the requested tx limit of " << limit;
-            " exceeds the requested tx limit of " << limit;
         return RPC::make_error (rpcHIGH_FEE, ss.str());
     }
 
@@ -364,7 +355,7 @@ enum class PathFind : unsigned char
 static Json::Value checkPayment(
     Json::Value const& params,
     Json::Value& tx_json,
-    AccountID const& raSrcAddressID,
+    AccountID const& srcAddressID,
     TxnSignApiFacade const& apiFacade,
     Role const role,
     PathFind const doPath)
@@ -399,21 +390,21 @@ static Json::Value checkPayment(
 
     if (!tx_json.isMember (jss::Paths) && params.isMember (jss::build_path))
     {
-        STAmount    saSendMax;
+        STAmount    sendMax;
 
         if (tx_json.isMember (jss::SendMax))
         {
-            if (! amountFromJsonNoThrow (saSendMax, tx_json [jss::SendMax]))
+            if (! amountFromJsonNoThrow (sendMax, tx_json [jss::SendMax]))
                 return RPC::invalid_field_error ("tx_json.SendMax");
         }
         else
         {
             // If no SendMax, default to Amount with sender as issuer.
-            saSendMax = amount;
+            sendMax = amount;
-            saSendMax.setIssuer (raSrcAddressID);
+            sendMax.setIssuer (srcAddressID);
         }
 
-        if (saSendMax.native () && amount.native ())
+        if (sendMax.native () && amount.native ())
             return RPC::make_error (rpcINVALID_PARAMS,
                 "Cannot build XRP to XRP paths.");
 
@@ -426,7 +417,7 @@ static Json::Value checkPayment(
             STPath fullLiquidityPath;
             bool valid = apiFacade.findPathsForOneIssuer (
                 *dstAccountID,
-                saSendMax.issue (),
+                sendMax.issue (),
                 amount,
                 getConfig ().PATH_SEARCH_OLD,
                 4,  // iMaxPaths
@@ -514,7 +505,7 @@ checkTxJsonFields (
         return ret;
     }
 
-    // It's all good.  Return the AddressID.
+    // It's all good.  Return the AccountID.
     ret.second = *srcAddressID;
    return ret;
 }
@@ -580,7 +571,7 @@ transactionPreProcessImpl (
     if (RPC::contains_error (txJsonResult.first))
         return std::move (txJsonResult.first);
 
-    auto const raSrcAddressID = txJsonResult.second;
+    auto const srcAddressID = txJsonResult.second;
 
     // This test covers the case where we're offline so the sequence number
     // cannot be determined locally.  If we're offline then the caller must
@@ -588,7 +579,7 @@ transactionPreProcessImpl (
     if (!verify && !tx_json.isMember (jss::Sequence))
         return RPC::missing_field_error ("tx_json.Sequence");
 
-    apiFacade.snapshotAccountState (raSrcAddressID);
+    apiFacade.snapshotAccountState (srcAddressID);
 
     if (verify) {
         if (!apiFacade.isValidAccount())
@@ -597,7 +588,7 @@ transactionPreProcessImpl (
             WriteLog (lsDEBUG, RPCHandler)
                 << "transactionSign: Failed to find source account "
                 << "in current ledger: "
-                << toBase58(raSrcAddressID);
+                << toBase58(srcAddressID);
 
             return rpcError (rpcSRC_ACT_NOT_FOUND);
         }
@@ -616,7 +607,7 @@ transactionPreProcessImpl (
         err = checkPayment (
             params,
             tx_json,
-            raSrcAddressID,
+            srcAddressID,
             apiFacade,
             role,
             signingArgs.editFields() ? PathFind::might : PathFind::dont);
@@ -640,9 +631,9 @@ transactionPreProcessImpl (
             // XXX Ignore transactions for accounts not created.
             return rpcError (rpcSRC_ACT_NOT_FOUND);
 
-        WriteLog (lsTRACE, RPCHandler) <<
+        WriteLog (lsTRACE, RPCHandler)
-            "verify: " << toBase58(calcAccountID(keypair.publicKey)) <<
+            << "verify: " << toBase58(calcAccountID(keypair.publicKey))
-            " : " << toBase58(raSrcAddressID);
+            << " : " << toBase58(srcAddressID);
 
         // If multisigning then we need to return the public key.
         if (signingArgs.isMultiSigning())
@@ -698,8 +689,8 @@ transactionPreProcessImpl (
     // If multisign then return multiSignature, else set TxnSignature field.
     if (signingArgs.isMultiSigning ())
     {
-        Serializer s = stpTrans->getMultiSigningData(
+        Serializer s =
-            signingArgs.getSigningFor(), signingArgs.getSigner());
+            buildMultiSigningData (*stpTrans, signingArgs.getSigner ());
         Blob multiSignature = keypair.secretKey.accountPrivateSign(s.getData());
         signingArgs.moveMultiSignature (std::move (multiSignature));
     }
@@ -797,51 +788,6 @@ Json::Value transactionFormatResultImpl (Transaction::pointer tpTrans)
     return jvResult;
 }
 
-void insertMultiSigners (
-    Json::Value& jvResult,
-    AccountID const& signingForAcctID,
-    AccountID const& signingAcctID,
-    RippleAddress const& accountPublic,
-    Blob const& signature)
-{
-    // Build a SigningAccount object to inject into the SigningAccounts.
-    Json::Value signingAccount (Json::objectValue);
-
-    signingAccount[sfAccount.getJsonName ()] =
-        getApp().accountIDCache().toBase58(signingAcctID);
-
-    signingAccount[sfSigningPubKey.getJsonName ()] =
-        strHex (accountPublic.getAccountPublic ());
-
-    signingAccount[sfMultiSignature.getJsonName ()] = strHex (signature);
-
-    // Give the SigningAccount an object name and put it in the
-    // SigningAccounts array.
-    Json::Value nameSigningAccount (Json::objectValue);
-    nameSigningAccount[sfSigningAccount.getJsonName ()] = signingAccount;
-
-    Json::Value signingAccounts (Json::arrayValue);
-    signingAccounts.append (nameSigningAccount);
-
-    // Put the signingForAcctID and the SigningAccounts in the SigningFor.
-    Json::Value signingFor (Json::objectValue);
-
-    signingFor[sfAccount.getJsonName ()] =
-        getApp().accountIDCache().toBase58(signingForAcctID);
-
-    signingFor[sfSigningAccounts.getJsonName ()] = signingAccounts;
-
-    // Give the SigningFor an object name and put it in the MultiSigners array.
-    Json::Value nameSigningFor (Json::objectValue);
-    nameSigningFor[sfSigningFor.getJsonName ()] = signingFor;
-
-    Json::Value multiSigners (Json::arrayValue);
-    multiSigners.append (nameSigningFor);
-
-    // Inject the MultiSigners into the jvResult.
-    jvResult[sfMultiSigners.getName ()] = multiSigners;
-}
-
 } // detail
 
 //------------------------------------------------------------------------------
@@ -953,8 +899,7 @@ Json::Value transactionSignFor (
     detail::TxnSignApiFacade& apiFacade,
     Role role)
 {
-    WriteLog (lsDEBUG, RPCHandler) <<
+    WriteLog (lsDEBUG, RPCHandler) << "transactionSignFor: " << jvRequest;
-        "transactionSignFor: " << jvRequest;
 
     // Verify presence of the signer's account field.
     const char accountField[] = "account";
@@ -962,30 +907,15 @@ Json::Value transactionSignFor (
     if (! jvRequest.isMember (accountField))
         return RPC::missing_field_error (accountField);
 
-    // Turn the signer's account into a RippleAddress for multi-sign.
+    // Turn the signer's account into an AccountID for multi-sign.
-    auto const multiSignAddrID = parseBase58<AccountID>(
+    auto const signerAccountID = parseBase58<AccountID>(
         jvRequest[accountField].asString());
-    if (! multiSignAddrID)
+    if (! signerAccountID)
     {
         return RPC::make_error (rpcSRC_ACT_MALFORMED,
             RPC::invalid_field_message (accountField));
     }
 
-    // Verify the presence of the "signing_for" field.
-    const char signing_forField[] = "signing_for";
-
-    if (! jvRequest.isMember (signing_forField))
-        return RPC::missing_field_error (signing_forField);
-
-    // Turn the signing_for account into a RippleAddress for multi-sign.
-    auto const multiSignForAddrID = parseBase58<AccountID>(
-        jvRequest[signing_forField].asString ());
-    if (! multiSignForAddrID)
-    {
-        return RPC::make_error (rpcSIGN_FOR_MALFORMED,
-            RPC::invalid_field_message (signing_forField));
-    }
-
     // When multi-signing, the "Sequence" and "SigningPubKey" fields must
     // be passed in by the caller.
     using namespace detail;
@@ -999,8 +929,7 @@ Json::Value transactionSignFor (
     Blob multiSignature;
     RippleAddress multiSignPubKey;
     SigningForParams signForParams(
-        *multiSignForAddrID, *multiSignAddrID,
+        *signerAccountID, multiSignPubKey, multiSignature);
-            multiSignPubKey, multiSignature);
 
     transactionPreProcessResult preprocResult =
         transactionPreProcessImpl (
@@ -1015,7 +944,7 @@ Json::Value transactionSignFor (
     // Make sure the multiSignAddrID can legitimately multi-sign.
     {
         error_code_i const err =
-            apiFacade.multiAcctMatchesPubKey(*multiSignAddrID, multiSignPubKey);
+            apiFacade.multiAcctMatchesPubKey (*signerAccountID, multiSignPubKey);
 
         if (err != rpcSUCCESS)
             return rpcError (err);
@@ -1032,9 +961,26 @@ Json::Value transactionSignFor (
     if (RPC::contains_error (json))
         return json;
 
-    // Finally, do what we were called for: return a SigningFor object.
+    // Finally, do what we were called for: return a Signers array.  Build
-    insertMultiSigners (json,
+    // a Signer object to insert into the Signers array.
-        *multiSignForAddrID, *multiSignAddrID, multiSignPubKey, multiSignature);
+    Json::Value signer (Json::objectValue);
+
+    signer[sfAccount.getJsonName ()] = toBase58 (*signerAccountID);
+
+    signer[sfSigningPubKey.getJsonName ()] =
+        strHex (multiSignPubKey.getAccountPublic ());
+
+    signer[sfTxnSignature.getJsonName ()] = strHex (multiSignature);
+
+    // Give the Signer an object name and put it in the Signers array.
+    Json::Value nameSigner (Json::objectValue);
+    nameSigner[sfSigner.getJsonName ()] = std::move (signer);
+
+    Json::Value signers (Json::arrayValue);
+    signers.append (std::move (nameSigner));
+
+    // Inject the Signers into the json.
+    json[sfSigners.getName ()] = std::move(signers);
 
     return json;
 }
@@ -1064,16 +1010,16 @@ Json::Value transactionSubmitMultiSigned (
     if (RPC::contains_error (txJsonResult.first))
         return std::move (txJsonResult.first);
 
-    auto const raSrcAddressID = txJsonResult.second;
+    auto const srcAddressID = txJsonResult.second;
 
-    apiFacade.snapshotAccountState (raSrcAddressID);
+    apiFacade.snapshotAccountState (srcAddressID);
     if (!apiFacade.isValidAccount ())
     {
         // If not offline and did not find account, error.
         WriteLog (lsDEBUG, RPCHandler)
             << "transactionSubmitMultiSigned: Failed to find source account "
             << "in current ledger: "
-            << toBase58(raSrcAddressID);
+            << toBase58(srcAddressID);
 
         return rpcError (rpcSRC_ACT_NOT_FOUND);
     }
@@ -1086,7 +1032,7 @@ Json::Value transactionSubmitMultiSigned (
         err = checkPayment (
             jvRequest,
             tx_json,
-            raSrcAddressID,
+            srcAddressID,
             apiFacade,
             role,
             PathFind::dont);
@@ -1147,146 +1093,81 @@ Json::Value transactionSubmitMultiSigned (
         }
     }
 
-    // Check MultiSigners for valid entries.
+    // Check Signers for valid entries.
-    const char* multiSignersArrayName {sfMultiSigners.getJsonName ().c_str ()};
+    STArray signers;
-
-    STArray multiSigners;
     {
-        // Verify that the MultiSigners field is present and an array.
+        // Verify that the Signers field is present and an array.
-        if (! jvRequest.isMember (multiSignersArrayName))
+        char const* signersArrayName {sfSigners.getJsonName ().c_str ()};
-            return RPC::missing_field_error (multiSignersArrayName);
+        if (! jvRequest.isMember (signersArrayName))
+            return RPC::missing_field_error (signersArrayName);
 
-        Json::Value& multiSignersValue (
+        Json::Value& signersValue (
-            jvRequest [multiSignersArrayName]);
+            jvRequest [signersArrayName]);
 
-        if (! multiSignersValue.isArray ())
+        if (! signersValue.isArray ())
         {
             std::ostringstream err;
             err << "Expected "
-                << multiSignersArrayName << " to be an array";
+                << signersArrayName << " to be an array";
             return RPC::make_param_error (err.str ());
         }
 
-        // Convert the MultiSigners into SerializedTypes.
+        // Convert signers into SerializedTypes.
-        STParsedJSONArray parsedMultiSigners (
+        STParsedJSONArray parsedSigners (signersArrayName, signersValue);
-            multiSignersArrayName, multiSignersValue);
 
-        if (!parsedMultiSigners.array)
+        if (!parsedSigners.array)
         {
             Json::Value jvResult;
-            jvResult ["error"] = parsedMultiSigners.error ["error"];
+            jvResult ["error"] = parsedSigners.error ["error"];
-            jvResult ["error_code"] =
+            jvResult ["error_code"] = parsedSigners.error ["error_code"];
-                parsedMultiSigners.error ["error_code"];
+            jvResult ["error_message"] = parsedSigners.error ["error_message"];
-            jvResult ["error_message"] =
-                parsedMultiSigners.error ["error_message"];
             return jvResult;
         }
-        multiSigners = std::move (parsedMultiSigners.array.get());
+        signers = std::move (parsedSigners.array.get());
     }
 
-    if (multiSigners.empty ())
+    if (signers.empty ())
-        return RPC::make_param_error ("MultiSigners array may not be empty.");
+        return RPC::make_param_error ("Signers array may not be empty.");
 
-    for (STObject const& signingFor : multiSigners)
+    // Signers must be sorted by Account.
+    signers.sort ([] (STObject const& a, STObject const& b)
     {
-        if (signingFor.getFName () != sfSigningFor)
+        return (a.getAccountID (sfAccount) < b.getAccountID (sfAccount));
-            return RPC::make_param_error (
+    });
-                "MultiSigners array has a non-SigningFor entry");
 
-        // Each SigningAccounts array must have at least one entry.
+    // Signers may not contain any duplicates.
-        STArray const& signingAccounts =
+    auto const dupIter = std::adjacent_find (
-            signingFor.getFieldArray (sfSigningAccounts);
+        signers.begin(), signers.end(),
-
+        [] (STObject const& a, STObject const& b)
-        if (signingAccounts.empty ())
-            return RPC::make_param_error (
-                "A SigningAccounts array may not be empty");
-
-        // Each SigningAccounts array may only contain SigningAccount objects.
-        auto const signingAccountsEnd = signingAccounts.end ();
-        auto const findItr = std::find_if (
-            signingAccounts.begin (), signingAccountsEnd,
-            [](STObject const& obj)
-            { return obj.getFName () != sfSigningAccount; });
-
-        if (findItr != signingAccountsEnd)
-            return RPC::make_param_error (
-                "SigningAccounts may only contain SigningAccount objects.");
-    }
-
-    // Lambdas for sorting arrays and finding duplicates.
-    auto byFieldAccountID = [](STObject const& a, STObject const& b)
         {
-            return a.getAccountID(sfAccount) <
+            return (a.getAccountID (sfAccount) == b.getAccountID (sfAccount));
-                b.getAccountID(sfAccount);
+        });
-        };
 
-    auto ifDuplicateAccountID =
+    if (dupIter != signers.end())
-        [](STObject const& a, STObject const& b)
-        {
-            return a.getAccountID(sfAccount) ==
-                b.getAccountID(sfAccount);
-        };
-
-    {
-        // MultiSigners are submitted sorted in AccountID order.  This
-        // assures that the same list will always have the same hash.
-        multiSigners.sort (byFieldAccountID);
-
-        // There may be no duplicate Accounts in MultiSigners
-        auto const multiSignersEnd = multiSigners.end ();
-
-        auto const dupAccountItr = std::adjacent_find (
-            multiSigners.begin (), multiSignersEnd, ifDuplicateAccountID);
-
-        if (dupAccountItr != multiSignersEnd)
     {
         std::ostringstream err;
-            err << "Duplicate SigningFor:Account entries ("
+        err << "Duplicate Signers:Signer:Account entries ("
-                << getApp().accountIDCache().toBase58(dupAccountItr->getAccountID(sfAccount))
+            << getApp().accountIDCache().toBase58(
-                << ") are not allowed.";
+                dupIter->getAccountID(sfAccount))
-            return RPC::make_param_error(err.str ());
-        }
-    }
-
-    // All SigningAccounts inside the MultiSigners must also be sorted and
-    // contain no duplicates.
-    for (STObject& signingFor : multiSigners)
-    {
-        STArray& signingAccts = signingFor.peekFieldArray (sfSigningAccounts);
-        signingAccts.sort (byFieldAccountID);
-
-        auto const signingAcctsEnd = signingAccts.end ();
-
-        auto const dupAccountItr = std::adjacent_find (
-            signingAccts.begin (), signingAcctsEnd, ifDuplicateAccountID);
-
-        if (dupAccountItr != signingAcctsEnd)
-        {
-            std::ostringstream err;
-            err << "Duplicate SigningAccounts:SigningAccount:Account entries ("
-                << getApp().accountIDCache().toBase58(dupAccountItr->getAccountID(sfAccount))
             << ") are not allowed.";
         return RPC::make_param_error(err.str ());
     }
 
     // An account may not sign for itself.
-        auto const account =
+    if (signers.end() != std::find_if (signers.begin(), signers.end(),
-            signingFor.getAccountID(sfAccount);
+        [&srcAddressID](STObject const& elem)
-        if (std::find_if (signingAccts.begin(), signingAcctsEnd,
-            [&account](STObject const& elem)
         {
-                    return elem.getAccountID(sfAccount) == account;
+            return elem.getAccountID (sfAccount) == srcAddressID;
-                }) != signingAcctsEnd)
+        }))
     {
         std::ostringstream err;
-            err << "A SigningAccount may not SignFor itself ("
+        err << "A Signer may not be the transaction's Account ("
-                << getApp().accountIDCache().toBase58(account) << ").";
+            << getApp().accountIDCache().toBase58(srcAddressID) << ").";
         return RPC::make_param_error(err.str ());
     }
-    }
 
-    // Insert the MultiSigners into the transaction.
+    // Insert signers into the transaction.
-    stpTrans->setFieldArray (sfMultiSigners, std::move(multiSigners));
+    stpTrans->setFieldArray (sfSigners, std::move(signers));
 
     // Make sure the SerializedTransaction makes a legitimate Transaction.
     std::pair <Json::Value, Transaction::pointer> txn =

src/ripple/rpc/tests/JSONRPC.test.cpp

@@ -80,14 +80,13 @@ R"({
 {
 "",
 "",
-"Missing field 'signing_for'.",
+"Missing field 'tx_json.Sequence'.",
 "Missing field 'tx_json.Sequence'."}},
 
 { "Pass in Sequence.",
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Sequence": 0,
@@ -107,7 +106,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Sequence": 0,
@@ -128,7 +126,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "fee_mult_max": 7,
     "tx_json": {
@@ -149,7 +146,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "fee_mult_max": 0,
     "tx_json": {
@@ -171,7 +167,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "fee_mult_max": "NotAFeeMultiplier",
     "tx_json": {
@@ -192,7 +187,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "fee_mult_max": 0,
     "tx_json": {
@@ -213,7 +207,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
@@ -231,7 +224,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
@@ -250,7 +242,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
@@ -268,7 +259,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
@@ -287,7 +277,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "build_path": 1,
     "tx_json": {
@@ -307,7 +296,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "build_path": 1,
     "tx_json": {
@@ -331,7 +319,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "build_path": 1,
     "tx_json": {
@@ -356,7 +343,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "build_path": 1,
     "tx_json": {
@@ -385,7 +371,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "build_path": 1,
     "tx_json": {
@@ -410,7 +395,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "tx_json": {
         "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
         "Amount": "1000000000",
@@ -428,7 +412,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "",
     "tx_json": {
         "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
@@ -447,7 +430,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "rx_json": {
         "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
@@ -466,7 +448,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
@@ -484,7 +465,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
@@ -503,7 +483,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
@@ -522,7 +501,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Amount": "1000000000",
@@ -540,7 +518,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "NotAnAccount",
@@ -559,7 +536,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "offline": 0,
     "tx_json": {
@@ -579,7 +555,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "offline": 1,
     "tx_json": {
@@ -599,7 +574,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "offline": 1,
     "tx_json": {
@@ -620,7 +594,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Flags": 0,
@@ -640,7 +613,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Flags": "NotGoodFlags",
@@ -660,7 +632,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "debug_signing": 0,
     "tx_json": {
@@ -680,7 +651,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
@@ -696,13 +666,12 @@ R"({
 "",
 "",
 "",
-"Missing field 'MultiSigners'."}},
+"Missing field 'Signers'."}},
 
 { "Missing 'Account' in sign_for.",
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Amount": "1000000000",
@@ -723,7 +692,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
@@ -744,7 +712,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
@@ -765,7 +732,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
@@ -786,7 +752,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
@@ -807,7 +772,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
@@ -828,7 +792,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
@@ -850,7 +813,6 @@ R"({
 R"({
     "command": "doesnt_matter",
     "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-    "signing_for": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
     "secret": "masterpassphrase",
     "tx_json": {
         "Account": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
@@ -870,21 +832,14 @@ R"({
 { "Minimal submit_multisigned.",
 R"({
     "command": "submit_multisigned",
-    "MultiSigners": [
+    "Signers": [
         {
-            "SigningFor": {
+            "Signer": {
-                "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
-                "SigningAccounts": [
-                    {
-                        "SigningAccount": {
                 "Account": "rPcNzota6B8YBokhYtcTNqQVCngtbnWfux",
-                            "MultiSignature": "3045022100F9ED357606932697A4FAB2BE7F222C21DD93CA4CFDD90357AADD07465E8457D6022038173193E3DFFFB5D78DD738CC0905395F885DA65B98FDB9793901FE3FD26ECE",
+                "TxnSignature": "3045022100F9ED357606932697A4FAB2BE7F222C21DD93CA4CFDD90357AADD07465E8457D6022038173193E3DFFFB5D78DD738CC0905395F885DA65B98FDB9793901FE3FD26ECE",
                 "SigningPubKey": "02FE36A690D6973D55F88553F5D2C4202DE75F2CF8A6D0E17C70AC223F044501F8"
             }
         }
-                ]
-            }
-        }
     ],
     "tx_json": {
         "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",

src/ripple/test/jtx/impl/Env_test.cpp

@@ -364,39 +364,8 @@ public:
         env(noop("alice"), msig("carol"));
         env(noop("alice"), msig("bob", "carol"));
         env(noop("alice"), msig("bob", "carol", "dilbert"),     ter(tefBAD_SIGNATURE));
-    }
 
-    // Two level Multi-sign
+        env(signers("alice", none));
-    void
-    testMultiSign2()
-    {
-        using namespace jtx;
-
-        Env env(*this);
-        env.fund(XRP(10000), "alice", "bob", "carol");
-        env.fund(XRP(10000), "david", "eric", "frank", "greg");
-        env(signers("alice", 2, { { "bob", 1 },   { "carol", 1 } }));
-        env(signers("bob",   1, { { "david", 1 }, { "eric", 1 } }));
-        env(signers("carol", 1, { { "frank", 1 }, { "greg", 1 } }));
-
-        env(noop("alice"), msig2(
-        { { "bob", "david" } }),                                ter(tefBAD_QUORUM));
-        
-        env(noop("alice"), msig2(
-        { { "bob", "david" }, { "bob", "eric" } }),             ter(tefBAD_QUORUM));
-
-        env(noop("alice"), msig2(
-        { { "carol", "frank" } }),                              ter(tefBAD_QUORUM));
-        
-        env(noop("alice"), msig2(
-        { { "carol", "frank" }, { "carol", "greg" } }),         ter(tefBAD_QUORUM));
-
-        env(noop("alice"), msig2(
-        { { "bob", "david" }, { "carol", "frank" } }));
-
-        env(noop("alice"), msig2(
-        { { "bob", "david" }, { "bob", "eric" },
-          { "carol", "frank" }, { "carol", "greg" } }));
     }
 
     void
@@ -581,7 +550,6 @@ public:
         testKeyType();
         testPayments();
         testMultiSign();
-        testMultiSign2();
         testTicket();
         testJTxProperties();
         testProp();

src/ripple/test/jtx/impl/multisign.cpp

@@ -22,6 +22,7 @@
 #include <ripple/test/jtx/utility.h>
 #include <ripple/protocol/HashPrefix.h>
 #include <ripple/protocol/JsonFields.h>
+#include <ripple/protocol/Sign.h>
 #include <ripple/protocol/types.h>
 
 namespace ripple {
@@ -36,15 +37,15 @@ signers (Account const& account,
     Json::Value jv;
     jv[jss::Account] = account.human();
     jv[jss::TransactionType] = "SignerListSet";
-    jv["SignerQuorum"] = quorum;
+    jv[sfSignerQuorum.getJsonName()] = quorum;
-    auto& ja = jv["SignerEntries"];
+    auto& ja = jv[sfSignerEntries.getJsonName()];
     ja.resize(v.size());
     for(std::size_t i = 0; i < v.size(); ++i)
     {
         auto const& e = v[i];
-        auto& je = ja[i]["SignerEntry"];
+        auto& je = ja[i][sfSignerEntry.getJsonName()];
         je[jss::Account] = e.account.human();
-        je["SignerWeight"] = e.weight;
+        je[sfSignerWeight.getJsonName()] = e.weight;
     }
     return jv;
 }
@@ -55,24 +56,30 @@ signers (Account const& account, none_t)
     Json::Value jv;
     jv[jss::Account] = account.human();
     jv[jss::TransactionType] = "SignerListSet";
+    jv[sfSignerQuorum.getJsonName()] = 0;
     return jv;
 }
 
 //------------------------------------------------------------------------------
 
+msig::msig (std::vector<msig::Reg> signers_)
+        : signers(std::move(signers_))
+{
+    // Signatures must be applied in sorted order.
+    std::sort(signers.begin(), signers.end(),
+        [](msig::Reg const& lhs, msig::Reg const& rhs)
+        {
+            return lhs.acct.id() < rhs.acct.id();
+        });
+}
+
 void
 msig::operator()(Env const& env, JTx& jt) const
 {
-    // VFALCO Inefficient pre-C++14
+    auto const mySigners = signers;
-    auto accounts = accounts_;
+    jt.signer = [mySigners, &env](Env&, JTx& jt)
-    std::sort(accounts.begin(), accounts.end(),
-        [](Account const& lhs, Account const& rhs)
     {
-            return lhs.id() < rhs.id();
+        jt[sfSigningPubKey.getJsonName()] = "";
-        });
-    jt.signer = [accounts, &env](Env&, JTx& jt)
-    {
-        jt["SigningPubKey"] = "";
         boost::optional<STObject> st;
         try
         {
@@ -83,107 +90,24 @@ msig::operator()(Env const& env, JTx& jt) const
             env.test.log << pretty(jt.jv);
             throw;
         }
-        auto const signFor =
+        auto& js = jt[sfSigners.getJsonName()];
-            parseBase58<AccountID>(
+        js.resize(mySigners.size());
-                jt.jv[jss::Account].asString());
+        for(std::size_t i = 0; i < mySigners.size(); ++i)
-        if (! signFor)
         {
-            env.test.log <<
+            auto const& e = mySigners[i];
-                "invalid AccountID: '" <<
+            auto& jo = js[i][sfSigner.getJsonName()];
-                    jt.jv[jss::Account].asString() << "'";
+            jo[jss::Account] = e.acct.human();
-            throw parse_error("msig: bad Account");
+            jo[jss::SigningPubKey] = strHex(e.sig.pk().slice());
-        }
-        auto& jv = jt["MultiSigners"][0u]["SigningFor"];
-        jv[jss::Account] = jt[jss::Account];
-        auto& js = jv["SigningAccounts"];
-        js.resize(accounts.size());
-        for(std::size_t i = 0; i < accounts.size(); ++i)
-        {
-            auto const& e = accounts[i];
-            auto& jo = js[i]["SigningAccount"];
-            jo[jss::Account] = e.human();
-            jo[jss::SigningPubKey] = strHex(e.pk().slice());
 
-            Serializer ss;
+            Serializer ss {buildMultiSigningData (*st, e.acct.id())};
-            ss.add32 (HashPrefix::txMultiSign);
+            auto const sig = ripple::sign (
-            st->addWithoutSigningFields(ss);
+                *publicKeyType(e.sig.pk().slice()), e.sig.sk(), ss.slice());
-            ss.add160(*signFor);
+            jo[sfTxnSignature.getJsonName()] =
-            ss.add160(e.id());
-            auto const sig = ripple::sign(
-                *publicKeyType(e.pk().slice()),
-                    e.sk(), ss.slice());
-            jo["MultiSignature"] =
             	strHex(Slice{ sig.data(), sig.size() });
         }
     };
 }
 
-msig2_t::msig2_t (std::vector<std::pair<
-    Account, Account>> sigs)
-{
-    for (auto& sig : sigs)
-    {
-        auto result = sigs_.emplace(
-            std::piecewise_construct,
-                std::make_tuple(std::move(sig.first)),
-                    std::make_tuple());
-        result.first->second.emplace(
-            std::move(sig.second));
-    }
-}
-
-void
-msig2_t::operator()(Env const& env, JTx& jt) const
-{
-    // VFALCO Inefficient pre-C++14
-    auto const sigs = sigs_;
-    jt.signer = [sigs, &env](Env&, JTx& jt)
-    {
-        jt["SigningPubKey"] = "";
-        boost::optional<STObject> st;
-        try
-        {
-            st = parse(jt.jv);
-        }
-        catch(parse_error const&)
-        {
-            env.test.log << pretty(jt.jv);
-            throw;
-        }
-        auto& ja = jt["MultiSigners"];
-        ja.resize(sigs.size());
-        for (auto i = std::make_pair(0, sigs.begin());
-            i.first < sigs.size(); ++i.first, ++i.second)
-        {
-            auto const& sign_for = i.second->first;
-            auto const& list = i.second->second;
-            auto& ji = ja[i.first]["SigningFor"];
-            ji[jss::Account] = sign_for.human();
-            auto& js = ji["SigningAccounts"];
-            js.resize(list.size());
-            for (auto j = std::make_pair(0, list.begin());
-                j.first < list.size(); ++j.first, ++j.second)
-            {
-                auto& jj = js[j.first]["SigningAccount"];
-                jj[jss::Account] = j.second->human();
-                jj[jss::SigningPubKey] = strHex(
-                    j.second->pk().slice());
-
-                Serializer ss;
-                ss.add32 (HashPrefix::txMultiSign);
-                st->addWithoutSigningFields(ss);
-                ss.add160(sign_for.id());
-                ss.add160(j.second->id());
-                auto const sig = ripple::sign(
-                    *publicKeyType(j.second->pk().slice()),
-                        j.second->sk(), ss.slice());
-                jj["MultiSignature"] =
-                    strHex(Slice{ sig.data(), sig.size() });
-            }
-        }
-    };
-}
-
 } // jtx
 } // test
 } // ripple

src/ripple/test/jtx/multisign.h

@@ -58,14 +58,42 @@ signers (Account const& account, none_t);
 /** Set a multisignature on a JTx. */
 class msig
 {
-private:
+public:
-    std::vector<Account> accounts_;
+    struct Reg
+    {
+        Account acct;
+        Account sig;
+
+        Reg (Account const& masterSig)
+        : acct (masterSig)
+        , sig (masterSig)
+        { }
+
+        Reg (Account const& acct_, Account const& regularSig)
+        : acct (acct_)
+        , sig (regularSig)
+        { }
+
+        Reg (char const* masterSig)
+        : acct (masterSig)
+        , sig (masterSig)
+        { }
+
+        Reg (char const* acct_, char const* regularSig)
+        : acct (acct_)
+        , sig (regularSig)
+        { }
+
+        bool operator< (Reg const& rhs) const
+        {
+            return acct < rhs.acct;
+        }
+    };
+
+    std::vector<Reg> signers;
 
 public:
-    msig (std::vector<Account> accounts)
+    msig (std::vector<Reg> signers_);
-        : accounts_(std::move(accounts))
-    {
-    }
 
     template <class AccountType, class... Accounts>
     msig (AccountType&& a0, Accounts&&... aN)
@@ -82,17 +110,17 @@ private:
     template <class AccountType>
     static
     void
-    helper (std::vector<Account>& v,
+    helper (std::vector<Reg>& v,
         AccountType&& account)
     {
         v.emplace_back(std::forward<
-            Account>(account));
+            Reg>(account));
     }
 
     template <class AccountType, class... Accounts>
     static
     void
-    helper (std::vector<Account>& v,
+    helper (std::vector<Reg>& v,
         AccountType&& a0, Accounts&&... aN)
     {
         helper(v, std::forward<AccountType>(a0));
@@ -101,44 +129,19 @@ private:
 
     template <class... Accounts>
     static
-    std::vector<Account>
+    std::vector<Reg>
-    make_vector(Accounts&&... accounts)
+    make_vector(Accounts&&... signers)
     {
-        std::vector<Account> v;
+        std::vector<Reg> v;
-        v.reserve(sizeof...(accounts));
+        v.reserve(sizeof...(signers));
         helper(v, std::forward<
-            Accounts>(accounts)...);
+            Accounts>(signers)...);
         return v;
     }
 };
 
 //------------------------------------------------------------------------------
 
-/** Set a multisignature on a JTx. */
-class msig2_t
-{
-private:
-    std::map<Account,
-        std::set<Account>> sigs_;
-
-public:
-    msig2_t (std::vector<std::pair<
-        Account, Account>> sigs);
-
-    void
-    operator()(Env const&, JTx& jt) const;
-};
-
-inline
-msig2_t
-msig2 (std::vector<std::pair<
-    Account, Account>> sigs)
-{
-    return msig2_t(std::move(sigs));
-}
-
-//------------------------------------------------------------------------------
-
 /** The number of signer lists matches. */
 using siglists = owner_count<ltSIGNER_LIST>;
 

src/ripple/unity/app_tx.cpp

@@ -43,7 +43,6 @@
 #include <ripple/app/tx/impl/TransactionAcquire.cpp>
 #include <ripple/app/tx/impl/Transactor.cpp>
 
-#include <ripple/app/tx/tests/common_transactor.cpp>
 #include <ripple/app/tx/tests/DeliverMin.test.cpp>
 #include <ripple/app/tx/tests/MultiSign.test.cpp>
 #include <ripple/app/tx/tests/OfferStream.test.cpp>