diff --git a/Builds/VisualStudio/stellar-core.vcxproj b/Builds/VisualStudio/stellar-core.vcxproj
index 26b857680e..0994484409 100644
--- a/Builds/VisualStudio/stellar-core.vcxproj
+++ b/Builds/VisualStudio/stellar-core.vcxproj
@@ -454,6 +454,8 @@ exit /b 0
+
+
@@ -678,6 +680,7 @@ exit /b 0
+
diff --git a/Builds/VisualStudio/stellar-core.vcxproj.filters b/Builds/VisualStudio/stellar-core.vcxproj.filters
index 2532a05a88..ef4f3cceb2 100644
--- a/Builds/VisualStudio/stellar-core.vcxproj.filters
+++ b/Builds/VisualStudio/stellar-core.vcxproj.filters
@@ -855,6 +855,12 @@
overlay
+
+ transactions\tests
+
+
+ transactions
+
@@ -1466,6 +1472,9 @@
database
+
+ transactions
+
main
diff --git a/src/herder/HerderTests.cpp b/src/herder/HerderTests.cpp
index cebf7a5dec..538703da6e 100644
--- a/src/herder/HerderTests.cpp
+++ b/src/herder/HerderTests.cpp
@@ -49,48 +49,118 @@ TEST_CASE("standalone", "[herder]")
auto root = TestAccount::createRoot(*app);
auto a1 = TestAccount{*app, getAccount("A")};
auto b1 = TestAccount{*app, getAccount("B")};
+ auto c1 = TestAccount{*app, getAccount("C")};
- const int64_t paymentAmount = app->getLedgerManager().getMinBalance(0);
+ auto txfee = app->getLedgerManager().getTxFee();
+ const int64_t minBalance = app->getLedgerManager().getMinBalance(0);
+ const int64_t paymentAmount = 100;
+ const int64_t startingBalance = minBalance + (paymentAmount + txfee) * 3;
SECTION("basic ledger close on valid txs")
{
- bool stop = false;
VirtualTimer setupTimer(*app);
- VirtualTimer checkTimer(*app);
-
- auto check = [&](asio::error_code const& error) {
- stop = true;
- REQUIRE(app->getLedgerManager().getLastClosedLedgerNum() > 2);
+ auto feedTx = [&](TransactionFramePtr& tx) {
+ REQUIRE(app->getHerder().recvTransaction(tx) ==
+ Herder::TX_STATUS_PENDING);
+ };
- AccountFrame::pointer a1Account, b1Account;
- a1Account = loadAccount(a1.getPublicKey(), *app);
- b1Account = loadAccount(b1.getPublicKey(), *app);
- REQUIRE(a1Account->getBalance() == paymentAmount);
- REQUIRE(b1Account->getBalance() == paymentAmount);
+ auto waitForExternalize = [&]() {
+ VirtualTimer checkTimer(*app);
+ bool stop = false;
+ auto prev = app->getLedgerManager().getLastClosedLedgerNum();
+
+ auto check = [&](asio::error_code const& error) {
+ REQUIRE(app->getLedgerManager().getLastClosedLedgerNum() >
+ prev);
+ stop = true;
+ };
+
+ checkTimer.expires_from_now(Herder::EXP_LEDGER_TIMESPAN_SECONDS +
+ std::chrono::seconds(1));
+ checkTimer.async_wait(check);
+ while (!stop)
+ {
+ app->getClock().crank(true);
+ }
};
auto setup = [&](asio::error_code const& error) {
// create accounts
- auto txFrameA1 = root.tx({createAccount(a1, paymentAmount)});
- auto txFrameA2 = root.tx({createAccount(b1, paymentAmount)});
+ auto txFrameA = root.tx({createAccount(a1, startingBalance)});
+ auto txFrameB = root.tx({createAccount(b1, startingBalance)});
+ auto txFrameC = root.tx({createAccount(c1, startingBalance)});
- REQUIRE(app->getHerder().recvTransaction(txFrameA1) ==
- Herder::TX_STATUS_PENDING);
- REQUIRE(app->getHerder().recvTransaction(txFrameA2) ==
- Herder::TX_STATUS_PENDING);
+ feedTx(txFrameA);
+ feedTx(txFrameB);
+ feedTx(txFrameC);
};
setupTimer.expires_from_now(std::chrono::seconds(0));
setupTimer.async_wait(setup);
- checkTimer.expires_from_now(Herder::EXP_LEDGER_TIMESPAN_SECONDS +
- std::chrono::seconds(1));
- checkTimer.async_wait(check);
-
- while (!stop)
- {
- app->getClock().crank(true);
+ waitForExternalize();
+ auto a1OldSeqNum = a1.getLastSequenceNumber();
+
+ REQUIRE(a1.getBalance() == startingBalance);
+ REQUIRE(b1.getBalance() == startingBalance);
+ REQUIRE(c1.getBalance() == startingBalance);
+
+ SECTION("txset with valid txs - but failing later")
+ {
+ std::vector txAs, txBs, txCs;
+ txAs.emplace_back(a1.tx({payment(root, paymentAmount)}));
+ txAs.emplace_back(a1.tx({payment(root, paymentAmount)}));
+ txAs.emplace_back(a1.tx({payment(root, paymentAmount)}));
+
+ txBs.emplace_back(b1.tx({payment(root, paymentAmount)}));
+ txBs.emplace_back(b1.tx({accountMerge(root)}));
+ txBs.emplace_back(b1.tx({payment(a1, paymentAmount)}));
+
+ auto expectedC1Seq = c1.getLastSequenceNumber() + 10;
+ txCs.emplace_back(c1.tx({payment(root, paymentAmount)}));
+ txCs.emplace_back(c1.tx({bumpSequence(expectedC1Seq)}));
+ txCs.emplace_back(c1.tx({payment(root, paymentAmount)}));
+
+ for_all_versions(*app, [&]() {
+ for (auto a : txAs)
+ {
+ feedTx(a);
+ }
+ for (auto b : txBs)
+ {
+ feedTx(b);
+ }
+
+ bool hasC =
+ app->getLedgerManager().getCurrentLedgerVersion() >= 10;
+ if (hasC)
+ {
+ for (auto c : txCs)
+ {
+ feedTx(c);
+ }
+ }
+
+ waitForExternalize();
+
+ // all of a1's transactions went through
+ // b1's last transaction failed due to account non existant
+ int64 expectedBalance =
+ startingBalance - 3 * paymentAmount - 3 * txfee;
+ REQUIRE(a1.getBalance() == expectedBalance);
+ REQUIRE(a1.loadSequenceNumber() == a1OldSeqNum + 3);
+ REQUIRE(!b1.exists());
+
+ if (hasC)
+ {
+ // c1's last transaction failed due to wrong sequence number
+ int64 expectedCBalance =
+ startingBalance - paymentAmount - 3 * txfee;
+ REQUIRE(c1.getBalance() == expectedCBalance);
+ REQUIRE(c1.loadSequenceNumber() == expectedC1Seq);
+ }
+ });
}
SECTION("Queue processing test")
diff --git a/src/invariant/LedgerEntryIsValid.cpp b/src/invariant/LedgerEntryIsValid.cpp
index fc39917c4e..7704a95b1c 100644
--- a/src/invariant/LedgerEntryIsValid.cpp
+++ b/src/invariant/LedgerEntryIsValid.cpp
@@ -106,15 +106,14 @@ LedgerEntryIsValid::checkIsValid(AccountEntry const& ae) const
{
return fmt::format("Account balance ({}) is negative", ae.balance);
}
- if (ae.seqNum > INT64_MAX)
+ if (ae.seqNum < 0)
{
- return fmt::format("Account seqNum ({}) exceeds limit ({})", ae.seqNum,
- INT64_MAX);
+ return fmt::format("Account seqNum ({}) is negative", ae.seqNum);
}
if (ae.numSubEntries > INT32_MAX)
{
return fmt::format("Account numSubEntries ({}) exceeds limit ({})",
- ae.seqNum, INT32_MAX);
+ ae.numSubEntries, INT32_MAX);
}
if ((ae.flags & ~MASK_ACCOUNT_FLAGS) != 0)
{
diff --git a/src/ledger/AccountFrame.cpp b/src/ledger/AccountFrame.cpp
index 50ab728d8f..cd2c69f3e0 100644
--- a/src/ledger/AccountFrame.cpp
+++ b/src/ledger/AccountFrame.cpp
@@ -248,7 +248,6 @@ AccountFrame::loadAccount(AccountID const& accountID, Database& db)
auto timer = db.getSelectTimer("account");
st.execute(true);
}
-
if (!st.got_data())
{
putCachedEntry(key, nullptr, db);
diff --git a/src/ledger/LedgerHeaderFrame.cpp b/src/ledger/LedgerHeaderFrame.cpp
index bbc216ade0..923af8fa32 100644
--- a/src/ledger/LedgerHeaderFrame.cpp
+++ b/src/ledger/LedgerHeaderFrame.cpp
@@ -61,10 +61,16 @@ LedgerHeaderFrame::getHash() const
return mHash;
}
+SequenceNumber
+LedgerHeaderFrame::getStartingSequenceNumber(uint32 ledgerSeq)
+{
+ return static_cast(ledgerSeq) << 32;
+}
+
SequenceNumber
LedgerHeaderFrame::getStartingSequenceNumber() const
{
- return static_cast(mHeader.ledgerSeq) << 32;
+ return getStartingSequenceNumber(mHeader.ledgerSeq);
}
uint64_t
diff --git a/src/ledger/LedgerHeaderFrame.h b/src/ledger/LedgerHeaderFrame.h
index d2a4cf466b..eea57f4dbe 100644
--- a/src/ledger/LedgerHeaderFrame.h
+++ b/src/ledger/LedgerHeaderFrame.h
@@ -35,6 +35,7 @@ class LedgerHeaderFrame
Hash const& getHash() const;
// returns the first sequence number to use for new accounts
+ static SequenceNumber getStartingSequenceNumber(uint32 ledgerSeq);
SequenceNumber getStartingSequenceNumber() const;
// methods to generate IDs
diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp
index ed2c61236e..e0451f59d6 100644
--- a/src/ledger/LedgerManagerImpl.cpp
+++ b/src/ledger/LedgerManagerImpl.cpp
@@ -932,25 +932,14 @@ LedgerManagerImpl::applyTransactions(std::vector& txs,
for (auto tx : txs)
{
auto txTime = mTransactionApply.TimeScope();
- LedgerDelta delta(ledgerDelta);
- TransactionMeta tm;
+ TransactionMeta tm(1);
try
{
CLOG(DEBUG, "Tx")
<< " tx#" << index << " = " << hexAbbrev(tx->getFullHash())
<< " txseq=" << tx->getSeqNum() << " (@ "
<< mApp.getConfig().toShortString(tx->getSourceID()) << ")";
-
- if (tx->apply(delta, tm, mApp))
- {
- delta.commit();
- }
- else
- {
- // failure means there should be no side effects
- assert(delta.getChanges().size() == 0);
- assert(delta.getHeader() == ledgerDelta.getHeader());
- }
+ tx->apply(ledgerDelta, tm.v1(), mApp);
}
catch (InvariantDoesNotHold& e)
{
diff --git a/src/ledger/LedgerTestUtils.cpp b/src/ledger/LedgerTestUtils.cpp
index 911b51fe6b..9d6ca56200 100644
--- a/src/ledger/LedgerTestUtils.cpp
+++ b/src/ledger/LedgerTestUtils.cpp
@@ -91,7 +91,10 @@ makeValid(AccountEntry& a)
}
}
a.numSubEntries = (uint32)a.signers.size();
- a.seqNum = a.seqNum & INT64_MAX;
+ if (a.seqNum < 0)
+ {
+ a.seqNum = -a.seqNum;
+ }
a.flags = a.flags & MASK_ACCOUNT_FLAGS;
}
diff --git a/src/main/Config.cpp b/src/main/Config.cpp
index 88a6d27849..9bbd225ebb 100644
--- a/src/main/Config.cpp
+++ b/src/main/Config.cpp
@@ -23,7 +23,7 @@ namespace stellar
{
using xdr::operator<;
-const uint32 Config::CURRENT_LEDGER_PROTOCOL_VERSION = 9;
+const uint32 Config::CURRENT_LEDGER_PROTOCOL_VERSION = 10;
Config::Config() : NODE_SEED(SecretKey::random())
{
diff --git a/src/simulation/LoadGenerator.cpp b/src/simulation/LoadGenerator.cpp
index cee1ad7aa7..113a3521ba 100644
--- a/src/simulation/LoadGenerator.cpp
+++ b/src/simulation/LoadGenerator.cpp
@@ -487,7 +487,8 @@ LoadGenerator::createAccount(size_t i, uint32_t ledgerNum)
auto accountName = "Account-" + to_string(i);
return make_shared(
i, txtest::getAccount(accountName.c_str()), 0,
- (static_cast(ledgerNum) << 32), ledgerNum, *this);
+ LedgerHeaderFrame::getStartingSequenceNumber(ledgerNum), ledgerNum,
+ *this);
}
vector
@@ -755,7 +756,7 @@ LoadGenerator::AccountInfo::createDirectly(Application& app)
AccountEntry& account = a.getAccount();
auto ledger = app.getLedgerManager().getLedgerNum();
account.balance = LOADGEN_ACCOUNT_BALANCE;
- account.seqNum = ((SequenceNumber)ledger) << 32;
+ account.seqNum = LedgerHeaderFrame::getStartingSequenceNumber(ledger);
a.touch(ledger);
LedgerDelta delta(app.getLedgerManager().getCurrentLedgerHeader(),
app.getDatabase());
diff --git a/src/test/TestAccount.cpp b/src/test/TestAccount.cpp
index 627f3f4bb7..f0a58c2420 100644
--- a/src/test/TestAccount.cpp
+++ b/src/test/TestAccount.cpp
@@ -16,17 +16,22 @@ namespace stellar
using namespace txtest;
SequenceNumber
-TestAccount::loadSequenceNumber() const
+TestAccount::loadSequenceNumber()
{
- return loadAccount(getPublicKey(), mApp)->getSeqNum();
+ mSn = 0;
+ return getLastSequenceNumber();
}
void
TestAccount::updateSequenceNumber()
{
- if (mSn == 0 && loadAccount(getPublicKey(), mApp, false))
+ if (mSn == 0)
{
- mSn = loadSequenceNumber();
+ auto a = loadAccount(getPublicKey(), mApp, false);
+ if (a)
+ {
+ mSn = a->getSeqNum();
+ }
}
}
@@ -36,6 +41,12 @@ TestAccount::getBalance() const
return loadAccount(getPublicKey(), mApp)->getBalance();
}
+bool
+TestAccount::exists() const
+{
+ return loadAccount(getPublicKey(), mApp, false) != nullptr;
+}
+
TransactionFramePtr
TestAccount::tx(std::vector const& ops, SequenceNumber sn)
{
@@ -174,6 +185,12 @@ TestAccount::manageData(std::string const& name, DataValue* value)
}
}
+void
+TestAccount::bumpSequence(SequenceNumber to)
+{
+ applyTx(tx({txtest::bumpSequence(to)}), mApp, false);
+}
+
OfferEntry
TestAccount::loadOffer(uint64_t offerID) const
{
@@ -222,7 +239,8 @@ TestAccount::pay(PublicKey const& destination, int64_t amount)
auto toAccountAfter = loadAccount(destination, mApp, false);
// check that the target account didn't change
REQUIRE(!!toAccount == !!toAccountAfter);
- if (toAccount && toAccountAfter)
+ if (toAccount && toAccountAfter &&
+ !(fromAccount->getID() == toAccount->getID()))
{
REQUIRE(toAccount->getAccount() == toAccountAfter->getAccount());
}
diff --git a/src/test/TestAccount.h b/src/test/TestAccount.h
index f29a966349..f64a7e6423 100644
--- a/src/test/TestAccount.h
+++ b/src/test/TestAccount.h
@@ -54,6 +54,8 @@ class TestAccount
void manageData(std::string const& name, DataValue* value);
+ void bumpSequence(SequenceNumber to);
+
OfferEntry loadOffer(uint64_t offerID) const;
bool hasOffer(uint64_t offerID) const;
@@ -113,10 +115,12 @@ class TestAccount
updateSequenceNumber();
return ++mSn;
}
- SequenceNumber loadSequenceNumber() const;
+ SequenceNumber loadSequenceNumber();
int64_t getBalance() const;
+ bool exists() const;
+
private:
Application& mApp;
SecretKey mSk;
diff --git a/src/test/TestExceptions.cpp b/src/test/TestExceptions.cpp
index ac05eac063..1ac7d75101 100644
--- a/src/test/TestExceptions.cpp
+++ b/src/test/TestExceptions.cpp
@@ -219,6 +219,8 @@ throwIf(AccountMergeResult const& result)
throw ex_ACCOUNT_MERGE_IMMUTABLE_SET{};
case ACCOUNT_MERGE_HAS_SUB_ENTRIES:
throw ex_ACCOUNT_MERGE_HAS_SUB_ENTRIES{};
+ case ACCOUNT_MERGE_SEQNUM_TOO_FAR:
+ throw ex_ACCOUNT_MERGE_SEQNUM_TOO_FAR{};
case ACCOUNT_MERGE_SUCCESS:
break;
default:
@@ -260,11 +262,30 @@ throwIf(ManageDataResult const& result)
}
}
+void
+throwIf(BumpSequenceResult const& result)
+{
+ switch (result.code())
+ {
+ case BUMP_SEQUENCE_SUCCESS:
+ break;
+ case BUMP_SEQUENCE_BAD_SEQ:
+ throw ex_BUMP_SEQUENCE_BAD_SEQ{};
+ default:
+ throw ex_UNKNOWN{};
+ }
+}
+
void
throwIf(TransactionResult const& result)
{
switch (result.result.code())
{
+ case txSUCCESS:
+ case txFAILED:
+ break;
+ case txBAD_SEQ:
+ throw ex_txBAD_SEQ{};
case txNO_ACCOUNT:
throw ex_txNO_ACCOUNT{};
case txINTERNAL_ERROR:
@@ -274,11 +295,27 @@ throwIf(TransactionResult const& result)
case txBAD_AUTH:
throw ex_txBAD_AUTH{};
default:
- // ignore rest for now
- break;
+ throw ex_UNKNOWN{};
}
auto opResult = result.result.results()[0];
+ switch (opResult.code())
+ {
+ case opINNER:
+ break;
+ case opBAD_AUTH:
+ throw ex_opBAD_AUTH{};
+ break;
+ case opNO_ACCOUNT:
+ throw ex_opNO_ACCOUNT{};
+ break;
+ case opNOT_SUPPORTED:
+ throw ex_opNOT_SUPPORTED{};
+ break;
+ default:
+ throw ex_UNKNOWN{};
+ };
+
switch (opResult.tr().type())
{
case CREATE_ACCOUNT:
@@ -314,6 +351,9 @@ throwIf(TransactionResult const& result)
case MANAGE_DATA:
throwIf(opResult.tr().manageDataResult());
break;
+ case BUMP_SEQUENCE:
+ throwIf(opResult.tr().bumpSeqResult());
+ break;
}
}
}
diff --git a/src/test/TestExceptions.h b/src/test/TestExceptions.h
index ee396a9975..cb44c69b44 100644
--- a/src/test/TestExceptions.h
+++ b/src/test/TestExceptions.h
@@ -23,16 +23,23 @@ class ex_txException
{ \
};
+TEST_EXCEPTION(ex_UNKNOWN)
+
+TEST_EXCEPTION(ex_txBAD_SEQ)
TEST_EXCEPTION(ex_txNO_ACCOUNT)
TEST_EXCEPTION(ex_txINTERNAL_ERROR)
TEST_EXCEPTION(ex_txINSUFFICIENT_BALANCE)
TEST_EXCEPTION(ex_txBAD_AUTH)
-TEST_EXCEPTION(ex_UNKNOWN)
+
+TEST_EXCEPTION(ex_opBAD_AUTH)
+TEST_EXCEPTION(ex_opNO_ACCOUNT)
+TEST_EXCEPTION(ex_opNOT_SUPPORTED)
TEST_EXCEPTION(ex_ACCOUNT_MERGE_MALFORMED)
TEST_EXCEPTION(ex_ACCOUNT_MERGE_NO_ACCOUNT)
TEST_EXCEPTION(ex_ACCOUNT_MERGE_IMMUTABLE_SET)
TEST_EXCEPTION(ex_ACCOUNT_MERGE_HAS_SUB_ENTRIES)
+TEST_EXCEPTION(ex_ACCOUNT_MERGE_SEQNUM_TOO_FAR)
TEST_EXCEPTION(ex_ALLOW_TRUST_MALFORMED)
TEST_EXCEPTION(ex_ALLOW_TRUST_NO_TRUST_LINE)
@@ -40,6 +47,8 @@ TEST_EXCEPTION(ex_ALLOW_TRUST_TRUST_NOT_REQUIRED)
TEST_EXCEPTION(ex_ALLOW_TRUST_CANT_REVOKE)
TEST_EXCEPTION(ex_ALLOW_TRUST_SELF_NOT_ALLOWED)
+TEST_EXCEPTION(ex_BUMP_SEQUENCE_BAD_SEQ)
+
TEST_EXCEPTION(ex_CREATE_ACCOUNT_MALFORMED)
TEST_EXCEPTION(ex_CREATE_ACCOUNT_UNDERFUNDED)
TEST_EXCEPTION(ex_CREATE_ACCOUNT_LOW_RESERVE)
diff --git a/src/test/TxTests.cpp b/src/test/TxTests.cpp
index 8ab9be9dc9..2dfd4d987a 100644
--- a/src/test/TxTests.cpp
+++ b/src/test/TxTests.cpp
@@ -15,6 +15,7 @@
#include "test/TestUtils.h"
#include "test/test.h"
#include "transactions/AllowTrustOpFrame.h"
+#include "transactions/BumpSequenceOpFrame.h"
#include "transactions/ChangeTrustOpFrame.h"
#include "transactions/CreateAccountOpFrame.h"
#include "transactions/InflationOpFrame.h"
@@ -41,48 +42,72 @@ namespace txtest
{
bool
-applyCheck(TransactionFramePtr tx, Application& app)
+applyCheck(TransactionFramePtr tx, Application& app, bool checkSeqNum)
{
app.getDatabase().clearPreparedStatementCache();
LedgerDelta delta(app.getLedgerManager().getCurrentLedgerHeader(),
app.getDatabase());
- auto txSet = std::make_shared(
- app.getLedgerManager().getLastClosedLedgerHeader().hash);
- txSet->add(tx);
-
- // TODO: maybe we should just close ledger with tx instead of checking all
- // of
- // that manually?
+ AccountEntry srcAccountBefore;
bool check = tx->checkValid(app, 0);
TransactionResult checkResult = tx->getResult();
REQUIRE((!check || checkResult.result.code() == txSUCCESS));
- bool doApply;
- // valid transaction sets ensure that
+ // now, check what happens when simulating what happens during a ledger
+ // close and reconcile it with the return value of "apply" with the one from
+ // checkValid:
+ // * an invalid (as per isValid) tx is still invalid during apply (and the
+ // same way)
+ // * a valid tx can fail later
+ auto code = checkResult.result.code();
+ if (code != txNO_ACCOUNT)
{
- auto code = checkResult.result.code();
- if (code != txNO_ACCOUNT && code != txBAD_SEQ && code != txBAD_AUTH)
- {
- tx->processFeeSeqNum(delta, app.getLedgerManager());
- doApply = true;
- }
- else
+ auto acnt = loadAccount(tx->getSourceID(), app, true);
+ srcAccountBefore = acnt->getAccount();
+
+ // no account -> can't process the fee
+ tx->processFeeSeqNum(delta, app.getLedgerManager());
+
+ // verify that the fee got processed
+ auto added = delta.added();
+ REQUIRE(added.begin() == added.end());
+ auto deleted = delta.deleted();
+ REQUIRE(deleted.begin() == deleted.end());
+ auto modified = delta.modified();
+ REQUIRE(modified.begin() != modified.end());
+ int modifiedCount = 0;
+ for (auto m : modified)
{
- doApply = (code != txBAD_SEQ);
+ modifiedCount++;
+ REQUIRE(modifiedCount == 1);
+ REQUIRE(m.key.account().accountID == tx->getSourceID());
+ auto& prevAccount = m.previous->mEntry.data.account();
+ REQUIRE(prevAccount == srcAccountBefore);
+ auto curAccount = m.current->mEntry.data.account();
+ // the balance should have changed
+ REQUIRE(curAccount.balance < prevAccount.balance);
+ curAccount.balance = prevAccount.balance;
+ if (app.getLedgerManager().getCurrentLedgerVersion() <= 9)
+ {
+ // v9 and below, we also need to verify that the sequence number
+ // also got processed at this time
+ REQUIRE(curAccount.seqNum == (prevAccount.seqNum + 1));
+ curAccount.seqNum = prevAccount.seqNum;
+ }
+ REQUIRE(curAccount == prevAccount);
}
}
bool res = false;
- if (doApply)
{
+ LedgerDelta applyDelta(delta);
try
{
- res = tx->apply(delta, app);
+ res = tx->apply(applyDelta, app);
}
catch (...)
{
@@ -91,14 +116,60 @@ applyCheck(TransactionFramePtr tx, Application& app)
REQUIRE((!res || tx->getResultCode() == txSUCCESS));
+ // checks that the failure is the same if pre checks failed
if (!check)
{
REQUIRE(checkResult == tx->getResult());
}
- }
- else
- {
- res = check;
+
+ if (code != txNO_ACCOUNT)
+ {
+ auto srcAccountAfter =
+ txtest::loadAccount(srcAccountBefore.accountID, app, false);
+ if (srcAccountAfter)
+ {
+ bool earlyFailure =
+ (code == txMISSING_OPERATION || code == txTOO_EARLY ||
+ code == txTOO_LATE || code == txINSUFFICIENT_FEE ||
+ code == txBAD_SEQ);
+ // verify that the sequence number changed (v10+)
+ // do not perform the check if there was a failure before
+ // or during the sequence number processing
+ if (checkSeqNum &&
+ app.getLedgerManager().getCurrentLedgerVersion() >= 10 &&
+ !earlyFailure)
+ {
+ REQUIRE(srcAccountAfter->getSeqNum() ==
+ (srcAccountBefore.seqNum + 1));
+ }
+ // on failure, no other changes should have been made
+ if (!res)
+ {
+ auto added = applyDelta.added();
+ REQUIRE(added.begin() == added.end());
+ auto deleted = applyDelta.deleted();
+ REQUIRE(deleted.begin() == deleted.end());
+ auto modified = applyDelta.modified();
+ if (earlyFailure ||
+ app.getLedgerManager().getCurrentLedgerVersion() <= 9)
+ {
+ // no changes during an early failure
+ REQUIRE(modified.begin() == modified.end());
+ }
+ else
+ {
+ REQUIRE(modified.begin() != modified.end());
+ for (auto m : modified)
+ {
+ REQUIRE(m.key.account().accountID ==
+ srcAccountBefore.accountID);
+ // could check more here if needed
+ }
+ }
+ }
+ }
+ }
+ applyDelta.commit();
}
// validates db state
@@ -118,9 +189,9 @@ checkTransaction(TransactionFrame& txFrame, Application& app)
}
void
-applyTx(TransactionFramePtr const& tx, Application& app)
+applyTx(TransactionFramePtr const& tx, Application& app, bool checkSeqNum)
{
- applyCheck(tx, app);
+ applyCheck(tx, app, checkSeqNum);
throwIf(tx->getResult());
checkTransaction(*tx, app);
}
@@ -626,6 +697,15 @@ manageData(std::string const& name, DataValue* value)
return op;
}
+Operation
+bumpSequence(SequenceNumber to)
+{
+ Operation op;
+ op.body.type(BUMP_SEQUENCE);
+ op.body.bumpSequenceOp().bumpTo = to;
+ return op;
+}
+
OperationFrame const&
getFirstOperationFrame(TransactionFrame const& tx)
{
diff --git a/src/test/TxTests.h b/src/test/TxTests.h
index d4f2f966b5..71d370f84e 100644
--- a/src/test/TxTests.h
+++ b/src/test/TxTests.h
@@ -34,8 +34,10 @@ struct ThresholdSetter
optional highThreshold;
};
-bool applyCheck(TransactionFramePtr tx, Application& app);
-void applyTx(TransactionFramePtr const& tx, Application& app);
+bool applyCheck(TransactionFramePtr tx, Application& app,
+ bool checkSeqNum = true);
+void applyTx(TransactionFramePtr const& tx, Application& app,
+ bool checkSeqNum = true);
TxSetResultMeta closeLedgerOn(Application& app, uint32 ledgerSeq, int day,
int month, int year,
@@ -77,6 +79,8 @@ Operation accountMerge(PublicKey const& dest);
Operation manageData(std::string const& name, DataValue* value);
+Operation bumpSequence(SequenceNumber to);
+
Operation createAccount(PublicKey const& dest, int64_t amount);
Operation payment(PublicKey const& to, int64_t amount);
diff --git a/src/transactions/BumpSequenceOpFrame.cpp b/src/transactions/BumpSequenceOpFrame.cpp
new file mode 100644
index 0000000000..ef65a4b460
--- /dev/null
+++ b/src/transactions/BumpSequenceOpFrame.cpp
@@ -0,0 +1,72 @@
+// Copyright 2017 Stellar Development Foundation and contributors. Licensed
+// under the Apache License, Version 2.0. See the COPYING file at the root
+// of this distribution or at http://www.apache.org/licenses/LICENSE-2.0
+
+#include "transactions/BumpSequenceOpFrame.h"
+#include "crypto/SignerKey.h"
+#include "database/Database.h"
+#include "main/Application.h"
+#include "medida/meter.h"
+#include "medida/metrics_registry.h"
+#include "transactions/TransactionFrame.h"
+
+namespace stellar
+{
+using xdr::operator==;
+
+BumpSequenceOpFrame::BumpSequenceOpFrame(Operation const& op,
+ OperationResult& res,
+ TransactionFrame& parentTx)
+ : OperationFrame(op, res, parentTx)
+ , mBumpSequenceOp(mOperation.body.bumpSequenceOp())
+{
+}
+
+ThresholdLevel
+BumpSequenceOpFrame::getThresholdLevel() const
+{
+ // bumping sequence is low threshold
+ return ThresholdLevel::LOW;
+}
+
+bool
+BumpSequenceOpFrame::isVersionSupported(uint32_t protocolVersion) const
+{
+ return protocolVersion >= 10;
+}
+
+bool
+BumpSequenceOpFrame::doApply(Application& app, LedgerDelta& delta,
+ LedgerManager& ledgerManager)
+{
+ SequenceNumber current = mSourceAccount->getSeqNum();
+
+ // Apply the bump (bump succeeds silently if bumpTo <= current)
+ if (mBumpSequenceOp.bumpTo > current)
+ {
+ mSourceAccount->setSeqNum(mBumpSequenceOp.bumpTo);
+ mSourceAccount->storeChange(delta, ledgerManager.getDatabase());
+ }
+
+ // Return successful results
+ innerResult().code(BUMP_SEQUENCE_SUCCESS);
+ app.getMetrics()
+ .NewMeter({"op-bump-sequence", "success", "apply"}, "operation")
+ .Mark();
+ return true;
+}
+
+bool
+BumpSequenceOpFrame::doCheckValid(Application& app)
+{
+ if (mBumpSequenceOp.bumpTo < 0)
+ {
+ app.getMetrics()
+ .NewMeter({"op-bump-sequence", "invalid", "bad-seq"}, "operation")
+ .Mark();
+ innerResult().code(BUMP_SEQUENCE_BAD_SEQ);
+ return false;
+ }
+ return true;
+}
+}
diff --git a/src/transactions/BumpSequenceOpFrame.h b/src/transactions/BumpSequenceOpFrame.h
new file mode 100644
index 0000000000..0589d74d43
--- /dev/null
+++ b/src/transactions/BumpSequenceOpFrame.h
@@ -0,0 +1,37 @@
+#pragma once
+
+// Copyright 2017 Stellar Development Foundation and contributors. Licensed
+// under the Apache License, Version 2.0. See the COPYING file at the root
+// of this distribution or at http://www.apache.org/licenses/LICENSE-2.0
+
+#include "transactions/OperationFrame.h"
+
+namespace stellar
+{
+class BumpSequenceOpFrame : public OperationFrame
+{
+ ThresholdLevel getThresholdLevel() const override;
+ bool isVersionSupported(uint32_t protocolVersion) const override;
+
+ BumpSequenceResult&
+ innerResult()
+ {
+ return mResult.tr().bumpSeqResult();
+ }
+ BumpSequenceOp const& mBumpSequenceOp;
+
+ public:
+ BumpSequenceOpFrame(Operation const& op, OperationResult& res,
+ TransactionFrame& parentTx);
+
+ bool doApply(Application& app, LedgerDelta& delta,
+ LedgerManager& ledgerManager) override;
+ bool doCheckValid(Application& app) override;
+
+ static BumpSequenceResultCode
+ getInnerCode(OperationResult const& res)
+ {
+ return res.tr().bumpSeqResult().code();
+ }
+};
+}
diff --git a/src/transactions/BumpSequenceTests.cpp b/src/transactions/BumpSequenceTests.cpp
new file mode 100644
index 0000000000..1188e61cac
--- /dev/null
+++ b/src/transactions/BumpSequenceTests.cpp
@@ -0,0 +1,80 @@
+// Copyright 2017 Stellar Development Foundation and contributors. Licensed
+// under the Apache License, Version 2.0. See the COPYING file at the root
+// of this distribution or at http://www.apache.org/licenses/LICENSE-2.0
+
+#include "crypto/SignerKey.h"
+#include "lib/catch.hpp"
+#include "main/Application.h"
+#include "main/Config.h"
+#include "overlay/LoopbackPeer.h"
+#include "test/TestAccount.h"
+#include "test/TestExceptions.h"
+#include "test/TestUtils.h"
+#include "test/TxTests.h"
+#include "test/test.h"
+#include "transactions/TransactionFrame.h"
+#include "util/Logging.h"
+#include "util/Timer.h"
+#include "util/make_unique.h"
+
+using namespace stellar;
+using namespace stellar::txtest;
+
+TEST_CASE("bump sequence", "[tx][bumpsequence]")
+{
+ using xdr::operator==;
+
+ Config const& cfg = getTestConfig();
+
+ VirtualClock clock;
+ auto app = createTestApplication(clock, cfg);
+ app->start();
+
+ // set up world
+ auto root = TestAccount::createRoot(*app);
+ auto& lm = app->getLedgerManager();
+
+ auto a = root.create("A", lm.getMinBalance(0) + 1000);
+ auto b = root.create("B", lm.getMinBalance(0) + 1000);
+
+ SECTION("test success")
+ {
+ for_versions_from(10, *app, [&]() {
+ SECTION("small bump")
+ {
+ auto newSeq = a.loadSequenceNumber() + 2;
+ a.bumpSequence(newSeq);
+ REQUIRE(a.loadSequenceNumber() == newSeq);
+ }
+ SECTION("large bump")
+ {
+ auto newSeq = INT64_MAX;
+ a.bumpSequence(newSeq);
+ REQUIRE(a.loadSequenceNumber() == newSeq);
+ SECTION("no more tx when INT64_MAX is reached")
+ {
+ REQUIRE_THROWS_AS(a.pay(root, 1), ex_txBAD_SEQ);
+ }
+ }
+ SECTION("backward jump (no-op)")
+ {
+ auto oldSeq = a.loadSequenceNumber();
+ a.bumpSequence(1);
+ // tx consumes sequence, bumpSequence doesn't do anything
+ REQUIRE(a.loadSequenceNumber() == oldSeq + 1);
+ }
+ SECTION("bad seq")
+ {
+ REQUIRE_THROWS_AS(a.bumpSequence(-1), ex_BUMP_SEQUENCE_BAD_SEQ);
+ REQUIRE_THROWS_AS(a.bumpSequence(INT64_MIN),
+ ex_BUMP_SEQUENCE_BAD_SEQ);
+ }
+ });
+ }
+ SECTION("not supported")
+ {
+ for_versions_to(9, *app, [&]() {
+ REQUIRE_THROWS_AS(a.bumpSequence(1), ex_opNOT_SUPPORTED);
+ });
+ }
+}
diff --git a/src/transactions/ChangeTrustTests.cpp b/src/transactions/ChangeTrustTests.cpp
index 6a8369623f..c9e32bfcc9 100644
--- a/src/transactions/ChangeTrustTests.cpp
+++ b/src/transactions/ChangeTrustTests.cpp
@@ -76,6 +76,7 @@ TEST_CASE("change trust", "[tx][changetrust]")
}
SECTION("edit existing")
{
+ closeLedgerOn(*app, 2, 1, 1, 2016);
for_all_versions(*app, [&] {
root.changeTrust(idr, 100);
// Merge gateway back into root (the trustline still exists)
diff --git a/src/transactions/MergeOpFrame.cpp b/src/transactions/MergeOpFrame.cpp
index e7842eda7c..5d9d800ab2 100644
--- a/src/transactions/MergeOpFrame.cpp
+++ b/src/transactions/MergeOpFrame.cpp
@@ -4,6 +4,7 @@
#include "transactions/MergeOpFrame.h"
#include "database/Database.h"
+#include "ledger/LedgerHeaderFrame.h"
#include "ledger/TrustFrame.h"
#include "main/Application.h"
#include "medida/meter.h"
@@ -92,6 +93,24 @@ MergeOpFrame::doApply(Application& app, LedgerDelta& delta,
return false;
}
+ if (ledgerManager.getCurrentLedgerVersion() >= 10)
+ {
+ SequenceNumber maxSeq = LedgerHeaderFrame::getStartingSequenceNumber(
+ ledgerManager.getCurrentLedgerHeader().ledgerSeq);
+
+ // don't allow the account to be merged if recreating it would cause it
+ // to jump backwards
+ if (mSourceAccount->getSeqNum() >= maxSeq)
+ {
+ app.getMetrics()
+ .NewMeter({"op-merge", "failure", "too-far"}, "operation")
+ .Mark();
+ innerResult().code(ACCOUNT_MERGE_SEQNUM_TOO_FAR);
+ return false;
+ }
+ }
+
+ // "success" path starts
if (!otherAccount->addBalance(sourceBalance))
{
throw std::runtime_error("merge overflowed destination balance");
diff --git a/src/transactions/MergeTests.cpp b/src/transactions/MergeTests.cpp
index 2f01f335d2..433484d27e 100644
--- a/src/transactions/MergeTests.cpp
+++ b/src/transactions/MergeTests.cpp
@@ -2,6 +2,7 @@
// under the Apache License, Version 2.0. See the COPYING file at the root
// of this distribution or at http://www.apache.org/licenses/LICENSE-2.0
+#include "crypto/SignerKey.h"
#include "ledger/LedgerManager.h"
#include "lib/catch.hpp"
#include "main/Application.h"
@@ -46,6 +47,11 @@ TEST_CASE("merge", "[tx][merge]")
app->getLedgerManager().getMinBalance(5) + 20 * txfee;
auto a1 = root.create("A", 2 * minBalance);
+ auto b1 = root.create("B", minBalance);
+ auto gateway = root.create("gate", minBalance);
+
+ // close ledger to allow a1, b1 and gateway to be merged in the next ledger
+ closeLedgerOn(*app, 2, 1, 1, 2016);
SECTION("merge into self")
{
@@ -57,14 +63,11 @@ TEST_CASE("merge", "[tx][merge]")
SECTION("merge into non existent account")
{
for_all_versions(*app, [&] {
- REQUIRE_THROWS_AS(a1.merge(getAccount("B").getPublicKey()),
+ REQUIRE_THROWS_AS(a1.merge(getAccount("C").getPublicKey()),
ex_ACCOUNT_MERGE_NO_ACCOUNT);
});
}
- auto b1 = root.create("B", minBalance);
- auto gateway = root.create("gate", minBalance);
-
SECTION("with create")
{
auto a1Balance = a1.getBalance();
@@ -89,7 +92,7 @@ TEST_CASE("merge", "[tx][merge]")
REQUIRE(!loadAccount(a1, *app, false));
});
- for_versions_from(6, *app, [&] {
+ for_versions(6, 9, *app, [&] {
applyCheck(txFrame, *app);
auto result = MergeOpFrame::getInnerCode(
@@ -100,13 +103,32 @@ TEST_CASE("merge", "[tx][merge]")
a1Balance + b1Balance - txFrame->getFee());
REQUIRE(!loadAccount(a1, *app, false));
});
+
+ for_versions_from(10, *app, [&]() {
+ // can't merge an account that just got created
+ REQUIRE(!applyCheck(txFrame, *app));
+ REQUIRE(txFrame->getResult()
+ .result.results()[0]
+ .tr()
+ .accountMergeResult()
+ .code() == ACCOUNT_MERGE_SUCCESS);
+ REQUIRE(txFrame->getResult()
+ .result.results()[1]
+ .tr()
+ .createAccountResult()
+ .code() == CREATE_ACCOUNT_SUCCESS);
+ REQUIRE(txFrame->getResult()
+ .result.results()[2]
+ .tr()
+ .accountMergeResult()
+ .code() == ACCOUNT_MERGE_SEQNUM_TOO_FAR);
+ });
}
SECTION("merge, create, merge back")
{
auto a1Balance = a1.getBalance();
auto b1Balance = b1.getBalance();
- auto a1SeqNum = a1.loadSequenceNumber();
auto createBalance = app->getLedgerManager().getMinBalance(1);
auto txFrame =
a1.tx({a1.op(accountMerge(b1)),
@@ -115,7 +137,8 @@ TEST_CASE("merge", "[tx][merge]")
txFrame->addSignature(b1.getSecretKey());
for_all_versions(*app, [&] {
- applyCheck(txFrame, *app);
+ // a1 gets re-created so we disable sequence number checks
+ applyCheck(txFrame, *app, false);
auto mergeResult = txFrame->getResult()
.result.results()[2]
@@ -127,7 +150,9 @@ TEST_CASE("merge", "[tx][merge]")
REQUIRE(a1.getBalance() ==
a1Balance + b1Balance - txFrame->getFee());
REQUIRE(!loadAccount(b1, *app, false));
- REQUIRE(a1SeqNum == a1.loadSequenceNumber());
+ // a1 gets recreated with a sequence number based on the current
+ // ledger
+ REQUIRE(a1.loadSequenceNumber() == 0x300000000ull);
});
}
@@ -429,43 +454,43 @@ TEST_CASE("merge", "[tx][merge]")
SECTION("With sub entries")
{
- Asset usd = makeAsset(gateway, "USD");
- a1.changeTrust(usd, trustLineLimit);
-
- SECTION("account has trust line")
+ SECTION("with trustline")
{
- for_all_versions(*app, [&] {
- REQUIRE_THROWS_AS(a1.merge(b1),
- ex_ACCOUNT_MERGE_HAS_SUB_ENTRIES);
- });
- }
- SECTION("account has offer")
- {
- for_all_versions(*app, [&] {
- gateway.pay(a1, usd, trustLineBalance);
- auto xlm = makeNativeAsset();
-
- const Price somePrice(3, 2);
- for (int i = 0; i < 4; i++)
- {
- a1.manageOffer(0, xlm, usd, somePrice, 100);
- }
- // empty out balance
- a1.pay(gateway, usd, trustLineBalance);
- // delete the trust line
- a1.changeTrust(usd, 0);
-
- REQUIRE_THROWS_AS(a1.merge(b1),
- ex_ACCOUNT_MERGE_HAS_SUB_ENTRIES);
- });
+ Asset usd = makeAsset(gateway, "USD");
+ a1.changeTrust(usd, trustLineLimit);
+
+ SECTION("account has trust line")
+ {
+ for_all_versions(*app, [&] {
+ REQUIRE_THROWS_AS(a1.merge(b1),
+ ex_ACCOUNT_MERGE_HAS_SUB_ENTRIES);
+ });
+ }
+ SECTION("account has offer")
+ {
+ for_all_versions(*app, [&] {
+ gateway.pay(a1, usd, trustLineBalance);
+ auto xlm = makeNativeAsset();
+
+ const Price somePrice(3, 2);
+ for (int i = 0; i < 4; i++)
+ {
+ a1.manageOffer(0, xlm, usd, somePrice, 100);
+ }
+ // empty out balance
+ a1.pay(gateway, usd, trustLineBalance);
+ // delete the trust line
+ a1.changeTrust(usd, 0);
+
+ REQUIRE_THROWS_AS(a1.merge(b1),
+ ex_ACCOUNT_MERGE_HAS_SUB_ENTRIES);
+ });
+ }
}
SECTION("account has data")
{
for_versions_from({2, 4}, *app, [&] {
- // delete the trust line
- a1.changeTrust(usd, 0);
-
DataValue value;
value.resize(20);
for (int n = 0; n < 20; n++)
@@ -480,6 +505,15 @@ TEST_CASE("merge", "[tx][merge]")
ex_ACCOUNT_MERGE_HAS_SUB_ENTRIES);
});
}
+ SECTION("account has signer")
+ {
+ for_all_versions(*app, [&]() {
+ Signer s(
+ KeyUtils::convertKey(gateway.getPublicKey()), 5);
+ a1.setOptions(nullptr, nullptr, nullptr, nullptr, &s, nullptr);
+ a1.merge(b1);
+ });
+ }
}
SECTION("success")
@@ -499,7 +533,7 @@ TEST_CASE("merge", "[tx][merge]")
auto tx2 = a1.tx({payment(root, 100)});
auto a1Balance = a1.getBalance();
auto b1Balance = b1.getBalance();
- auto r = closeLedgerOn(*app, 2, 1, 1, 2015, {tx1, tx2});
+ auto r = closeLedgerOn(*app, 3, 1, 1, 2017, {tx1, tx2});
checkTx(0, r, txSUCCESS);
checkTx(1, r, txNO_ACCOUNT);
@@ -545,6 +579,7 @@ TEST_CASE("merge", "[tx][merge]")
{
auto mergeFrom = root.create(
"merge-from", app->getLedgerManager().getMinBalance(0) + txfee);
+ closeLedgerOn(*app, 3, 1, 1, 2017);
for_versions_to(8, *app, [&] {
REQUIRE_THROWS_AS(mergeFrom.merge(root), ex_txINSUFFICIENT_BALANCE);
});
@@ -556,6 +591,7 @@ TEST_CASE("merge", "[tx][merge]")
{
auto mergeFrom = root.create(
"merge-from", app->getLedgerManager().getMinBalance(0) + txfee + 1);
+ closeLedgerOn(*app, 3, 1, 1, 2017);
for_versions_to(8, *app, [&] {
REQUIRE_THROWS_AS(mergeFrom.merge(root), ex_txINSUFFICIENT_BALANCE);
});
@@ -568,6 +604,7 @@ TEST_CASE("merge", "[tx][merge]")
auto mergeFrom =
root.create("merge-from", app->getLedgerManager().getMinBalance(0) +
2 * txfee - 1);
+ closeLedgerOn(*app, 3, 1, 1, 2017);
for_versions_to(8, *app, [&] {
REQUIRE_THROWS_AS(mergeFrom.merge(root), ex_txINSUFFICIENT_BALANCE);
});
@@ -579,6 +616,40 @@ TEST_CASE("merge", "[tx][merge]")
{
auto mergeFrom = root.create(
"merge-from", app->getLedgerManager().getMinBalance(0) + 2 * txfee);
+ closeLedgerOn(*app, 3, 1, 1, 2017);
for_all_versions(*app, [&] { mergeFrom.merge(root); });
}
+
+ SECTION("merge too far")
+ {
+ for_versions_from(10, *app, [&]() {
+ SequenceNumber curStartSeqNum =
+ LedgerHeaderFrame::getStartingSequenceNumber(
+ app->getLedgerManager().getLedgerNum());
+ auto maxSeqNum = curStartSeqNum - 1;
+
+ auto txFrame = root.tx({a1.op(accountMerge(b1))});
+ txFrame->addSignature(a1.getSecretKey());
+
+ SECTION("at max = success")
+ {
+ a1.bumpSequence(maxSeqNum);
+ REQUIRE(a1.loadSequenceNumber() == maxSeqNum);
+ REQUIRE(applyCheck(txFrame, *app));
+ }
+ SECTION("passed max = failure")
+ {
+ maxSeqNum++;
+ a1.bumpSequence(maxSeqNum);
+ REQUIRE(a1.loadSequenceNumber() == maxSeqNum);
+
+ REQUIRE(!applyCheck(txFrame, *app));
+ REQUIRE(txFrame->getResult()
+ .result.results()[0]
+ .tr()
+ .accountMergeResult()
+ .code() == ACCOUNT_MERGE_SEQNUM_TOO_FAR);
+ }
+ });
+ }
}
diff --git a/src/transactions/OperationFrame.cpp b/src/transactions/OperationFrame.cpp
index 0352a4d654..3b61b1eb79 100644
--- a/src/transactions/OperationFrame.cpp
+++ b/src/transactions/OperationFrame.cpp
@@ -8,6 +8,7 @@
#include "ledger/LedgerDelta.h"
#include "main/Application.h"
#include "transactions/AllowTrustOpFrame.h"
+#include "transactions/BumpSequenceOpFrame.h"
#include "transactions/ChangeTrustOpFrame.h"
#include "transactions/CreateAccountOpFrame.h"
#include "transactions/CreatePassiveOfferOpFrame.h"
@@ -79,7 +80,8 @@ OperationFrame::makeHelper(Operation const& op, OperationResult& res,
return std::make_shared(op, res, tx);
case MANAGE_DATA:
return std::make_shared(op, res, tx);
-
+ case BUMP_SEQUENCE:
+ return std::make_shared(op, res, tx);
default:
ostringstream err;
err << "Unknown Tx type: " << op.body.type();
@@ -113,6 +115,11 @@ OperationFrame::getThresholdLevel() const
return ThresholdLevel::MEDIUM;
}
+bool OperationFrame::isVersionSupported(uint32_t) const
+{
+ return true;
+}
+
bool
OperationFrame::checkSignature(SignatureChecker& signatureChecker) const
{
@@ -153,6 +160,15 @@ OperationFrame::checkValid(SignatureChecker& signatureChecker, Application& app,
LedgerDelta* delta)
{
bool forApply = (delta != nullptr);
+ if (!isVersionSupported(app.getLedgerManager().getCurrentLedgerVersion()))
+ {
+ app.getMetrics()
+ .NewMeter({"operation", "invalid", "not-supported"}, "operation")
+ .Mark();
+ mResult.code(opNOT_SUPPORTED);
+ return false;
+ }
+
if (!loadAccount(app.getLedgerManager().getCurrentLedgerVersion(), delta,
app.getDatabase()))
{
diff --git a/src/transactions/OperationFrame.h b/src/transactions/OperationFrame.h
index 92daffc9ce..bc06849bc0 100644
--- a/src/transactions/OperationFrame.h
+++ b/src/transactions/OperationFrame.h
@@ -44,8 +44,12 @@ class OperationFrame
virtual bool doCheckValid(Application& app) = 0;
virtual bool doApply(Application& app, LedgerDelta& delta,
LedgerManager& ledgerManager) = 0;
+ // returns the threshold this operation requires
virtual ThresholdLevel getThresholdLevel() const;
+ // returns true if the operation is supported given a protocol version
+ virtual bool isVersionSupported(uint32_t protocolVersion) const;
+
public:
static std::shared_ptr
makeHelper(Operation const& op, OperationResult& res,
diff --git a/src/transactions/PathPaymentTests.cpp b/src/transactions/PathPaymentTests.cpp
index 8065ff0086..22244da598 100644
--- a/src/transactions/PathPaymentTests.cpp
+++ b/src/transactions/PathPaymentTests.cpp
@@ -124,6 +124,8 @@ TEST_CASE("pathpayment", "[tx][pathpayment]")
auto cur3 = makeAsset(gateway2, "CUR3");
auto cur4 = makeAsset(gateway2, "CUR4");
+ closeLedgerOn(*app, 2, 1, 1, 2016);
+
SECTION("path payment destination amount 0")
{
auto market = TestMarket{*app};
diff --git a/src/transactions/PaymentTests.cpp b/src/transactions/PaymentTests.cpp
index 34fe3b2c45..7a3266e7a2 100644
--- a/src/transactions/PaymentTests.cpp
+++ b/src/transactions/PaymentTests.cpp
@@ -90,6 +90,8 @@ TEST_CASE("payment", "[tx][payment]")
REQUIRE(rootAccount->getBalance() == (1000000000000000000 - paymentAmount -
gatewayPayment * 2 - txfee * 3));
+ closeLedgerOn(*app, 2, 1, 1, 2016);
+
SECTION("Create account")
{
SECTION("Success")
@@ -133,6 +135,7 @@ TEST_CASE("payment", "[tx][payment]")
int64 a1Balance = a1.getBalance();
int64 b1Balance = b1.getBalance();
+ closeLedgerOn(*app, 3, 1, 2, 2016);
auto txFrame = a1.tx({payment(b1, 200), accountMerge(b1)});
@@ -159,6 +162,8 @@ TEST_CASE("payment", "[tx][payment]")
int64 a1Balance = a1.getBalance();
int64 b1Balance = b1.getBalance();
+ closeLedgerOn(*app, 3, 1, 2, 2016);
+
auto txFrame = a1.tx({payment(b1, 200), b1.op(accountMerge(a1))});
txFrame->addSignature(b1);
@@ -182,6 +187,8 @@ TEST_CASE("payment", "[tx][payment]")
int64 a1Balance = a1.getBalance();
int64 b1Balance = b1.getBalance();
+ closeLedgerOn(*app, 3, 1, 2, 2016);
+
auto txFrame = a1.tx({accountMerge(b1), payment(b1, 200)});
for_versions_to(7, *app, [&] {
@@ -278,7 +285,7 @@ TEST_CASE("payment", "[tx][payment]")
auto rootBalance = root.getBalance();
for_versions_to(8, *app, [&] {
- auto r = closeLedgerOn(*app, 2, 1, 1, 2015, {tx1, tx2});
+ auto r = closeLedgerOn(*app, 3, 1, 2, 2016, {tx1, tx2});
checkTx(0, r, txSUCCESS);
checkTx(1, r, txINSUFFICIENT_BALANCE);
@@ -290,7 +297,7 @@ TEST_CASE("payment", "[tx][payment]")
});
for_versions_from(9, *app, [&] {
- auto r = closeLedgerOn(*app, 2, 1, 1, 2015, {tx1, tx2});
+ auto r = closeLedgerOn(*app, 3, 1, 2, 2016, {tx1, tx2});
checkTx(0, r, txSUCCESS);
checkTx(1, r, txFAILED);
REQUIRE(r[1].first.result.result.results()[0]
@@ -315,6 +322,8 @@ TEST_CASE("payment", "[tx][payment]")
auto createSourceAccount = TestAccount{*app, getAccount("create")};
auto sourceSeqNum = sourceAccount.getLastSequenceNumber();
+ closeLedgerOn(*app, 3, 1, 2, 2016);
+
auto tx =
sourceAccount.tx({createAccount(createSourceAccount, createAmount),
accountMerge(createSourceAccount),
@@ -418,7 +427,7 @@ TEST_CASE("payment", "[tx][payment]")
REQUIRE(tx->getResult().result.code() == txINTERNAL_ERROR);
});
- for_versions_from(8, *app, [&] {
+ for_versions(8, 9, *app, [&] {
REQUIRE(!applyCheck(tx, *app));
REQUIRE(loadAccount(sourceAccount, *app));
REQUIRE(!loadAccount(createSourceAccount, *app, false));
@@ -452,6 +461,7 @@ TEST_CASE("payment", "[tx][payment]")
amount - createAmount - tx->getFee());
REQUIRE(tx->getResult().result.results()[2].code() == opNO_ACCOUNT);
});
+ // 10 and above, operation #2 fails (already covered in merge tests)
}
SECTION("pay, merge, create, pay, self")
@@ -462,10 +472,11 @@ TEST_CASE("payment", "[tx][payment]")
auto pay2Amount = 200000000;
auto sourceAccount = root.create("source", amount);
auto payAndMergeDestination = root.create("payAndMerge", amount);
- auto sourceSeqNum = sourceAccount.getLastSequenceNumber();
auto payAndMergeDestinationSeqNum =
payAndMergeDestination.getLastSequenceNumber();
+ closeLedgerOn(*app, 3, 1, 2, 2016);
+
auto tx =
sourceAccount.tx({payAndMergeDestination.op(
payment(payAndMergeDestination, pay1Amount)),
@@ -483,7 +494,7 @@ TEST_CASE("payment", "[tx][payment]")
REQUIRE(sourceAccount.getBalance() == createAmount);
REQUIRE(payAndMergeDestination.getBalance() ==
amount + amount - createAmount - tx->getFee());
- REQUIRE(sourceAccount.loadSequenceNumber() == sourceSeqNum);
+ REQUIRE(sourceAccount.loadSequenceNumber() == 0x400000000ull);
REQUIRE(payAndMergeDestination.loadSequenceNumber() ==
payAndMergeDestinationSeqNum);
@@ -526,13 +537,15 @@ TEST_CASE("payment", "[tx][payment]")
});
for_versions_from(8, *app, [&] {
- REQUIRE(applyCheck(tx, *app));
+ // as the account gets re-created we have to disable seqnum
+ // verification
+ REQUIRE(applyCheck(tx, *app, false));
REQUIRE(loadAccount(sourceAccount, *app));
REQUIRE(loadAccount(payAndMergeDestination, *app));
REQUIRE(sourceAccount.getBalance() == createAmount);
REQUIRE(payAndMergeDestination.getBalance() ==
amount + amount - createAmount - tx->getFee());
- REQUIRE(sourceAccount.loadSequenceNumber() == sourceSeqNum);
+ REQUIRE(sourceAccount.loadSequenceNumber() == 0x400000000ull);
REQUIRE(payAndMergeDestination.loadSequenceNumber() ==
payAndMergeDestinationSeqNum);
@@ -587,6 +600,8 @@ TEST_CASE("payment", "[tx][payment]")
auto payAndMergeDestinationSeqNum =
payAndMergeDestination.getLastSequenceNumber();
+ closeLedgerOn(*app, 3, 1, 2, 2016);
+
auto tx =
sourceAccount.tx({payment(payAndMergeDestination, pay1Amount),
accountMerge(payAndMergeDestination),
@@ -647,13 +662,15 @@ TEST_CASE("payment", "[tx][payment]")
});
for_versions_from(8, *app, [&] {
- REQUIRE(applyCheck(tx, *app));
+ // as the account gets re-created we have to disable seqnum
+ // verification
+ REQUIRE(applyCheck(tx, *app, false));
REQUIRE(loadAccount(sourceAccount, *app));
REQUIRE(loadAccount(payAndMergeDestination, *app));
REQUIRE(sourceAccount.getBalance() == createAmount - pay2Amount);
REQUIRE(payAndMergeDestination.getBalance() ==
amount + amount + pay2Amount - tx->getFee() - createAmount);
- REQUIRE(sourceAccount.loadSequenceNumber() == sourceSeqNum);
+ REQUIRE(sourceAccount.loadSequenceNumber() == 0x400000000ull);
REQUIRE(payAndMergeDestination.loadSequenceNumber() ==
payAndMergeDestinationSeqNum);
@@ -711,6 +728,8 @@ TEST_CASE("payment", "[tx][payment]")
auto payAndMergeDestinationSeqNum =
payAndMergeDestination.getLastSequenceNumber();
+ closeLedgerOn(*app, 3, 1, 2, 2016);
+
auto tx = sourceAccount.tx(
{payment(payAndMergeDestination, pay1Amount),
accountMerge(payAndMergeDestination),
@@ -774,7 +793,9 @@ TEST_CASE("payment", "[tx][payment]")
});
for_versions_from(8, *app, [&] {
- REQUIRE(applyCheck(tx, *app));
+ // as the account gets re-created we have to disable seqnum
+ // verification
+ REQUIRE(applyCheck(tx, *app, false));
REQUIRE(loadAccount(sourceAccount, *app));
REQUIRE(loadAccount(secondSourceAccount, *app));
REQUIRE(loadAccount(payAndMergeDestination, *app));
@@ -782,7 +803,7 @@ TEST_CASE("payment", "[tx][payment]")
REQUIRE(secondSourceAccount.getBalance() == amount - createAmount);
REQUIRE(payAndMergeDestination.getBalance() ==
amount + amount + pay2Amount - tx->getFee());
- REQUIRE(sourceAccount.loadSequenceNumber() == sourceSeqNum);
+ REQUIRE(sourceAccount.loadSequenceNumber() == 0x400000000ull);
REQUIRE(secondSourceAccount.loadSequenceNumber() ==
secondSourceSeqNum);
REQUIRE(payAndMergeDestination.loadSequenceNumber() ==
@@ -841,7 +862,8 @@ TEST_CASE("payment", "[tx][payment]")
auto payDestination = root.create("pay", amount);
auto sourceSeqNum = sourceAccount.getLastSequenceNumber();
auto createSourceSeqNum = createSource.getLastSequenceNumber();
- auto payDestinationSeqNum = payDestination.getLastSequenceNumber();
+
+ closeLedgerOn(*app, 3, 1, 2, 2016);
auto tx = sourceAccount.tx({
createSource.op(createAccount(createDestination, create1Amount)),
@@ -869,8 +891,8 @@ TEST_CASE("payment", "[tx][payment]")
REQUIRE(payDestination.getBalance() == create2Amount);
REQUIRE(sourceAccount.loadSequenceNumber() == sourceSeqNum + 1);
REQUIRE(createSource.loadSequenceNumber() == createSourceSeqNum);
- REQUIRE(payDestination.loadSequenceNumber() ==
- payDestinationSeqNum);
+ REQUIRE(createDestination.loadSequenceNumber() == 0x400000000ull);
+ REQUIRE(payDestination.loadSequenceNumber() == 0x400000000ull);
REQUIRE(tx->getResult().result.code() == txSUCCESS);
REQUIRE(tx->getResult().result.results()[0].code() == opINNER);
@@ -923,6 +945,8 @@ TEST_CASE("payment", "[tx][payment]")
auto sourceSeqNum = sourceAccount.getLastSequenceNumber();
auto mergeDestinationSeqNum = mergeDestination.getLastSequenceNumber();
+ closeLedgerOn(*app, 3, 1, 2, 2016);
+
auto tx = sourceAccount.tx({payment(sourceAccount, pay1Amount),
accountMerge(mergeDestination),
payment(sourceAccount, pay2Amount),
@@ -1064,6 +1088,8 @@ TEST_CASE("payment", "[tx][payment]")
auto sourceSeqNum = sourceAccount.getLastSequenceNumber();
auto mergeDestinationSeqNum = mergeDestination.getLastSequenceNumber();
+ closeLedgerOn(*app, 3, 1, 2, 2016);
+
auto tx = sourceAccount.tx({payment(sourceAccount, pay1Amount),
payment(sourceAccount, pay1Amount),
payment(sourceAccount, pay1Amount),
diff --git a/src/transactions/TransactionFrame.cpp b/src/transactions/TransactionFrame.cpp
index 315b844a9b..b16d1b23c8 100644
--- a/src/transactions/TransactionFrame.cpp
+++ b/src/transactions/TransactionFrame.cpp
@@ -210,11 +210,10 @@ TransactionFrame::resetResults()
}
bool
-TransactionFrame::commonValid(SignatureChecker& signatureChecker,
- Application& app, LedgerDelta* delta,
- SequenceNumber current)
+TransactionFrame::commonValidPreSeqNum(Application& app, LedgerDelta* delta)
{
- bool applying = (delta != nullptr);
+ // this function does validations that are independent of the account state
+ // (stay true regardless of other side effects)
if (mOperations.size() == 0)
{
@@ -270,24 +269,59 @@ TransactionFrame::commonValid(SignatureChecker& signatureChecker,
return false;
}
- // when applying, the account's sequence number is updated when taking fees
- if (!applying)
+ return true;
+}
+
+void
+TransactionFrame::processSeqNum(LedgerManager& lm, LedgerDelta& delta)
+{
+ if (lm.getCurrentLedgerVersion() >= 10)
+ {
+ if (mSigningAccount->getSeqNum() > mEnvelope.tx.seqNum)
+ {
+ throw std::runtime_error("unexpected sequence number");
+ }
+ mSigningAccount->setSeqNum(mEnvelope.tx.seqNum);
+ mSigningAccount->storeChange(delta, lm.getDatabase());
+ }
+}
+
+TransactionFrame::ValidationType
+TransactionFrame::commonValid(SignatureChecker& signatureChecker,
+ Application& app, LedgerDelta* delta,
+ SequenceNumber current)
+{
+ ValidationType res = ValidationType::kInvalid;
+
+ bool applying = (delta != nullptr);
+
+ if (!commonValidPreSeqNum(app, delta))
+ {
+ return res;
+ }
+
+ auto& lm = app.getLedgerManager();
+
+ // in older versions, the account's sequence number is updated when taking
+ // fees
+ if (lm.getCurrentLedgerVersion() >= 10 || !applying)
{
if (current == 0)
{
current = mSigningAccount->getSeqNum();
}
-
- if (current + 1 != mEnvelope.tx.seqNum)
+ if (current == INT64_MAX || current + 1 != mEnvelope.tx.seqNum)
{
app.getMetrics()
.NewMeter({"transaction", "invalid", "bad-seq"}, "transaction")
.Mark();
getResult().result.code(txBAD_SEQ);
- return false;
+ return res;
}
}
+ res = ValidationType::kInvalidUpdateSeqNum;
+
if (!checkSignature(signatureChecker, *mSigningAccount,
mSigningAccount->getLowThreshold()))
{
@@ -295,30 +329,29 @@ TransactionFrame::commonValid(SignatureChecker& signatureChecker,
.NewMeter({"transaction", "invalid", "bad-auth"}, "transaction")
.Mark();
getResult().result.code(txBAD_AUTH);
- return false;
+ return res;
}
// if we are in applying mode fee was already deduced from signing account
// balance, if not, we need to check if after that deduction this account
// will still have minimum balance
auto balanceAfter =
- (applying && (app.getLedgerManager().getCurrentLedgerVersion() > 8))
+ (applying && (lm.getCurrentLedgerVersion() > 8))
? mSigningAccount->getAccount().balance
: mSigningAccount->getAccount().balance - mEnvelope.tx.fee;
// don't let the account go below the reserve
- if (balanceAfter <
- mSigningAccount->getMinimumBalance(app.getLedgerManager()))
+ if (balanceAfter < mSigningAccount->getMinimumBalance(lm))
{
app.getMetrics()
.NewMeter({"transaction", "invalid", "insufficient-balance"},
"transaction")
.Mark();
getResult().result.code(txINSUFFICIENT_BALANCE);
- return false;
+ return res;
}
- return true;
+ return ValidationType::kFullyValid;
}
void
@@ -343,13 +376,17 @@ TransactionFrame::processFeeSeqNum(LedgerDelta& delta,
mSigningAccount->addBalance(-fee);
delta.getHeader().feePool += fee;
}
- if (mSigningAccount->getSeqNum() + 1 != mEnvelope.tx.seqNum)
+ // in v10 we update sequence numbers during apply
+ if (ledgerManager.getCurrentLedgerVersion() <= 9)
{
- // this should not happen as the transaction set is sanitized for
- // sequence numbers
- throw std::runtime_error("Unexpected account state");
+ if (mSigningAccount->getSeqNum() + 1 != mEnvelope.tx.seqNum)
+ {
+ // this should not happen as the transaction set is sanitized for
+ // sequence numbers
+ throw std::runtime_error("Unexpected account state");
+ }
+ mSigningAccount->setSeqNum(mEnvelope.tx.seqNum);
}
- mSigningAccount->setSeqNum(mEnvelope.tx.seqNum);
mSigningAccount->storeChange(delta, db);
}
@@ -424,7 +461,8 @@ TransactionFrame::checkValid(Application& app, SequenceNumber current)
SignatureChecker signatureChecker{
app.getLedgerManager().getCurrentLedgerVersion(), getContentsHash(),
mEnvelope.signatures};
- bool res = commonValid(signatureChecker, app, nullptr, current);
+ bool res = commonValid(signatureChecker, app, nullptr, current) ==
+ ValidationType::kFullyValid;
if (res)
{
for (auto& op : mOperations)
@@ -481,30 +519,22 @@ TransactionFrame::markResultFailed()
bool
TransactionFrame::apply(LedgerDelta& delta, Application& app)
{
- TransactionMeta tm;
- return apply(delta, tm, app);
+ TransactionMeta tm(1);
+ return apply(delta, tm.v1(), app);
}
bool
-TransactionFrame::apply(LedgerDelta& delta, TransactionMeta& meta,
- Application& app)
+TransactionFrame::applyOperations(SignatureChecker& signatureChecker,
+ LedgerDelta& delta, TransactionMetaV1& meta,
+ Application& app)
{
- resetSigningAccount();
- SignatureChecker signatureChecker{
- app.getLedgerManager().getCurrentLedgerVersion(), getContentsHash(),
- mEnvelope.signatures};
- if (!commonValid(signatureChecker, app, &delta, 0))
- {
- return false;
- }
-
bool errorEncountered = false;
{
// shield outer scope of any side effects by using
// a sql transaction for ledger state and LedgerDelta
soci::transaction sqlTx(app.getDatabase().getSession());
- LedgerDelta thisTxDelta(delta);
+ LedgerDelta thisTxOpsDelta(delta);
auto& opTimer =
app.getMetrics().NewTimer({"transaction", "op", "apply"});
@@ -512,7 +542,7 @@ TransactionFrame::apply(LedgerDelta& delta, TransactionMeta& meta,
for (auto& op : mOperations)
{
auto time = opTimer.TimeScope();
- LedgerDelta opDelta(thisTxDelta);
+ LedgerDelta opDelta(thisTxOpsDelta);
bool txRes = op->apply(signatureChecker, opDelta, app);
if (!txRes)
@@ -524,7 +554,7 @@ TransactionFrame::apply(LedgerDelta& delta, TransactionMeta& meta,
app.getInvariantManager().checkOnOperationApply(
op->getOperation(), op->getResult(), opDelta);
}
- meta.operations().emplace_back(opDelta.getChanges());
+ meta.operations.emplace_back(opDelta.getChanges());
opDelta.commit();
}
@@ -540,22 +570,49 @@ TransactionFrame::apply(LedgerDelta& delta, TransactionMeta& meta,
// if an error occurred, it is responsibility of account's owner to
// remove that signer
- removeUsedOneTimeSignerKeys(signatureChecker, thisTxDelta,
+ removeUsedOneTimeSignerKeys(signatureChecker, thisTxOpsDelta,
app.getLedgerManager());
sqlTx.commit();
- thisTxDelta.commit();
+ thisTxOpsDelta.commit();
}
}
if (errorEncountered)
{
- meta.operations().clear();
+ meta.operations.clear();
markResultFailed();
}
return !errorEncountered;
}
+bool
+TransactionFrame::apply(LedgerDelta& delta, TransactionMetaV1& meta,
+ Application& app)
+{
+ resetSigningAccount();
+ SignatureChecker signatureChecker{
+ app.getLedgerManager().getCurrentLedgerVersion(), getContentsHash(),
+ mEnvelope.signatures};
+
+ bool valid;
+ {
+ LedgerDelta txDelta(delta);
+ // when applying, a failure during tx validation means that
+ // we'll skip trying to apply operations but we'll still
+ // process the sequence number if needed
+ auto cv = commonValid(signatureChecker, app, &txDelta, 0);
+ if (cv >= ValidationType::kInvalidUpdateSeqNum)
+ {
+ processSeqNum(app.getLedgerManager(), txDelta);
+ }
+ meta.txChanges = txDelta.getChanges();
+ txDelta.commit();
+ valid = (cv == ValidationType::kFullyValid);
+ }
+ return valid && applyOperations(signatureChecker, delta, meta, app);
+}
+
StellarMessage
TransactionFrame::toStellarMessage() const
{
diff --git a/src/transactions/TransactionFrame.h b/src/transactions/TransactionFrame.h
index 28580748b8..eaece9e311 100644
--- a/src/transactions/TransactionFrame.h
+++ b/src/transactions/TransactionFrame.h
@@ -51,8 +51,19 @@ class TransactionFrame
bool loadAccount(int ledgerProtocolVersion, LedgerDelta* delta,
Database& app);
- bool commonValid(SignatureChecker& signatureChecker, Application& app,
- LedgerDelta* delta, SequenceNumber current);
+
+ enum ValidationType
+ {
+ kInvalid, // transaction is not valid at all
+ kInvalidUpdateSeqNum, // transaction is invalid but its sequence number
+ // should be updated
+ kFullyValid
+ };
+
+ bool commonValidPreSeqNum(Application& app, LedgerDelta* delta);
+ ValidationType commonValid(SignatureChecker& signatureChecker,
+ Application& app, LedgerDelta* delta,
+ SequenceNumber current);
void resetSigningAccount();
void resetResults();
@@ -68,6 +79,11 @@ class TransactionFrame
LedgerManager& ledgerManager) const;
void markResultFailed();
+ bool applyOperations(SignatureChecker& checker, LedgerDelta& delta,
+ TransactionMetaV1& meta, Application& app);
+
+ void processSeqNum(LedgerManager& lm, LedgerDelta& delta);
+
public:
TransactionFrame(Hash const& networkID,
TransactionEnvelope const& envelope);
@@ -147,7 +163,7 @@ class TransactionFrame
// apply this transaction to the current ledger
// returns true if successfully applied
- bool apply(LedgerDelta& delta, TransactionMeta& meta, Application& app);
+ bool apply(LedgerDelta& delta, TransactionMetaV1& meta, Application& app);
// version without meta
bool apply(LedgerDelta& delta, Application& app);
diff --git a/src/transactions/TxEnvelopeTests.cpp b/src/transactions/TxEnvelopeTests.cpp
index 1ac18fccc3..12bc9856ce 100644
--- a/src/transactions/TxEnvelopeTests.cpp
+++ b/src/transactions/TxEnvelopeTests.cpp
@@ -326,7 +326,6 @@ TEST_CASE("txenvelope", "[tx][envelope]")
ex_SET_OPTIONS_BAD_SIGNER);
});
- testutil::setCurrentLedgerVersion(app->getLedgerManager(), 3);
SECTION("single signature")
{
SECTION("invalid seq nr")
@@ -343,7 +342,12 @@ TEST_CASE("txenvelope", "[tx][envelope]")
REQUIRE(getAccountSigners(a1, *app).size() == 1);
alternative.sign(*tx);
- for_versions_from(3, *app, [&] {
+ for_versions(3, 9, *app, [&] {
+ REQUIRE(!tx->checkValid(*app, 0));
+ REQUIRE(tx->getResultCode() == txBAD_SEQ);
+ REQUIRE(getAccountSigners(a1, *app).size() == 1);
+ });
+ for_versions_from(10, *app, [&] {
applyCheck(tx, *app);
REQUIRE(tx->getResultCode() == txBAD_SEQ);
REQUIRE(getAccountSigners(a1, *app).size() == 1);
@@ -438,9 +442,11 @@ TEST_CASE("txenvelope", "[tx][envelope]")
SECTION("accountMerge signing account")
{
- auto b1 = root.create("a1", paymentAmount);
+ auto b1 = root.create("b1", paymentAmount);
a1.pay(b1, 1000);
+ closeLedgerOn(*app, 2, 1, 1, 2016);
+
auto tx = b1.tx({accountMerge(a1)},
b1.getLastSequenceNumber() + 2);
tx->getEnvelope().signatures.clear();
@@ -676,8 +682,6 @@ TEST_CASE("txenvelope", "[tx][envelope]")
SECTION("empty X")
{
- testutil::setCurrentLedgerVersion(app->getLedgerManager(), 3);
-
SecretKey s1 = getAccount("S1");
Signer sk1(KeyUtils::convertKey(s1.getPublicKey()), 95);
@@ -966,7 +970,11 @@ TEST_CASE("txenvelope", "[tx][envelope]")
SECTION("duplicate payment")
{
- for_all_versions(*app, [&] {
+ for_versions_to(9, *app, [&] {
+ REQUIRE(!txFrame->checkValid(*app, 0));
+ REQUIRE(txFrame->getResultCode() == txBAD_SEQ);
+ });
+ for_versions_from(10, *app, [&] {
applyCheck(txFrame, *app);
REQUIRE(txFrame->getResultCode() == txBAD_SEQ);
@@ -985,39 +993,56 @@ TEST_CASE("txenvelope", "[tx][envelope]")
clock.setCurrentTime(ledgerTime);
- txFrame =
- root.tx({payment(a1.getPublicKey(), paymentAmount)});
- txFrame->getEnvelope().tx.timeBounds.activate() =
- TimeBounds(start + 1000, start + 10000);
-
- closeLedgerOn(*app, 3, 1, 7, 2014);
- applyCheck(txFrame, *app);
-
- REQUIRE(txFrame->getResultCode() == txTOO_EARLY);
+ SECTION("too early")
+ {
+ txFrame = root.tx(
+ {payment(a1.getPublicKey(), paymentAmount)});
+ txFrame->getEnvelope().tx.timeBounds.activate() =
+ TimeBounds(start + 1000, start + 10000);
- txFrame =
- root.tx({payment(a1.getPublicKey(), paymentAmount)});
- txFrame->getEnvelope().tx.timeBounds.activate() =
- TimeBounds(1000, start + 300000);
+ closeLedgerOn(*app, 3, 1, 7, 2014);
+ applyCheck(txFrame, *app);
- closeLedgerOn(*app, 4, 2, 7, 2014);
- applyCheck(txFrame, *app);
- REQUIRE(txFrame->getResultCode() == txSUCCESS);
+ REQUIRE(txFrame->getResultCode() == txTOO_EARLY);
+ }
- txFrame =
- root.tx({payment(a1.getPublicKey(), paymentAmount)});
- txFrame->getEnvelope().tx.timeBounds.activate() =
- TimeBounds(1000, start);
+ SECTION("on time")
+ {
+ txFrame = root.tx(
+ {payment(a1.getPublicKey(), paymentAmount)});
+ txFrame->getEnvelope().tx.timeBounds.activate() =
+ TimeBounds(1000, start + 300000);
+
+ closeLedgerOn(*app, 3, 2, 7, 2014);
+ applyCheck(txFrame, *app);
+ REQUIRE(txFrame->getResultCode() == txSUCCESS);
+ }
- closeLedgerOn(*app, 5, 3, 7, 2014);
- applyCheck(txFrame, *app);
- REQUIRE(txFrame->getResultCode() == txTOO_LATE);
+ SECTION("too late")
+ {
+ txFrame = root.tx(
+ {payment(a1.getPublicKey(), paymentAmount)});
+ txFrame->getEnvelope().tx.timeBounds.activate() =
+ TimeBounds(1000, start);
+
+ closeLedgerOn(*app, 3, 3, 7, 2014);
+ applyCheck(txFrame, *app);
+ REQUIRE(txFrame->getResultCode() == txTOO_LATE);
+ }
});
}
SECTION("transaction gap")
{
- for_all_versions(*app, [&] {
+ for_versions_to(9, *app, [&] {
+ txFrame =
+ root.tx({payment(a1.getPublicKey(), paymentAmount)});
+ txFrame->getEnvelope().tx.seqNum--;
+ REQUIRE(!txFrame->checkValid(*app, 0));
+
+ REQUIRE(txFrame->getResultCode() == txBAD_SEQ);
+ });
+ for_versions_from(10, *app, [&] {
txFrame =
root.tx({payment(a1.getPublicKey(), paymentAmount)});
txFrame->getEnvelope().tx.seqNum--;
diff --git a/src/transactions/TxResultsTests.cpp b/src/transactions/TxResultsTests.cpp
index 5600296c71..b310bb502a 100644
--- a/src/transactions/TxResultsTests.cpp
+++ b/src/transactions/TxResultsTests.cpp
@@ -674,6 +674,7 @@ TEST_CASE("txresults", "[tx][txresults]")
SECTION("merge account")
{
+ closeLedgerOn(*app, 2, 1, 1, 2016);
SECTION("normal")
{
auto tx = a.tx({payment(b, 1000), accountMerge(root)});
diff --git a/src/transactions/readme.md b/src/transactions/readme.md
index ae5dc9a070..9f7653076f 100644
--- a/src/transactions/readme.md
+++ b/src/transactions/readme.md
@@ -62,9 +62,12 @@ transaction; therefore this is really a best effort implementation specific.
## Applying a transaction
When SCP externalizes the transaction set to apply to the last closed ledger:
-Source Accounts for transactions are charged a fee and their sequence number
-is updated.
-Then, the transactions are applied one by one.
+1. the Source accounts for all transactions are charged a fee
+2. transactions are applied one by one, checking and updating the account's
+ sequence number.
+
+note that in earlier versions of the protocol (9 and below), the sequence
+ number was updated at the same time than when the fee was charged.
To apply a transaction, it first checks for part __A__ of the validity check.
diff --git a/src/util/types.h b/src/util/types.h
index ce9911715b..0816b05ac0 100644
--- a/src/util/types.h
+++ b/src/util/types.h
@@ -6,6 +6,7 @@
#include "overlay/StellarXDR.h"
#include "xdrpp/message.h"
+#include
#include
namespace stellar
diff --git a/src/xdr/Stellar-ledger-entries.x b/src/xdr/Stellar-ledger-entries.x
index c2969c1f6c..ddbb8d0124 100644
--- a/src/xdr/Stellar-ledger-entries.x
+++ b/src/xdr/Stellar-ledger-entries.x
@@ -11,7 +11,7 @@ typedef PublicKey AccountID;
typedef opaque Thresholds[4];
typedef string string32<32>;
typedef string string64<64>;
-typedef uint64 SequenceNumber;
+typedef int64 SequenceNumber;
typedef opaque DataValue<64>;
enum AssetType
diff --git a/src/xdr/Stellar-ledger.x b/src/xdr/Stellar-ledger.x
index 6556ed360c..5021112c8a 100644
--- a/src/xdr/Stellar-ledger.x
+++ b/src/xdr/Stellar-ledger.x
@@ -265,9 +265,19 @@ struct OperationMeta
LedgerEntryChanges changes;
};
+struct TransactionMetaV1
+{
+ LedgerEntryChanges txChanges; // tx level changes if any
+ OperationMeta operations<>; // meta for each operation
+};
+
+// this is the meta produced when applying transactions
+// it does not include pre-apply updates such as fees
union TransactionMeta switch (int v)
{
case 0:
OperationMeta operations<>;
+case 1:
+ TransactionMetaV1 v1;
};
}
diff --git a/src/xdr/Stellar-transaction.x b/src/xdr/Stellar-transaction.x
index ad4c912116..ff5176c72a 100644
--- a/src/xdr/Stellar-transaction.x
+++ b/src/xdr/Stellar-transaction.x
@@ -25,7 +25,8 @@ enum OperationType
ALLOW_TRUST = 7,
ACCOUNT_MERGE = 8,
INFLATION = 9,
- MANAGE_DATA = 10
+ MANAGE_DATA = 10,
+ BUMP_SEQUENCE = 11
};
/* CreateAccount
@@ -207,8 +208,8 @@ Result: InflationResult
*/
/* ManageData
- Adds, Updates, or Deletes a key value pair associated with a particular
- account.
+ Adds, Updates, or Deletes a key value pair associated with a particular
+ account.
Threshold: med
@@ -217,8 +218,20 @@ Result: InflationResult
struct ManageDataOp
{
- string64 dataName;
- DataValue* dataValue; // set to null to clear
+ string64 dataName;
+ DataValue* dataValue; // set to null to clear
+};
+
+/* Bump Sequence
+
+ increases the sequence to a given level
+
+ Result: BumpSequenceResult
+*/
+
+struct BumpSequenceOp
+{
+ SequenceNumber bumpTo;
};
/* An operation is the lowest unit of work that a transaction does */
@@ -253,6 +266,8 @@ struct Operation
void;
case MANAGE_DATA:
ManageDataOp manageDataOp;
+ case BUMP_SEQUENCE:
+ BumpSequenceOp bumpSequenceOp;
}
body;
};
@@ -321,14 +336,16 @@ struct Transaction
ext;
};
-struct TransactionSignaturePayload {
+struct TransactionSignaturePayload
+{
Hash networkId;
union switch (EnvelopeType type)
{
case ENVELOPE_TYPE_TX:
- Transaction tx;
- /* All other values of type are invalid */
- } taggedTransaction;
+ Transaction tx;
+ /* All other values of type are invalid */
+ }
+ taggedTransaction;
};
/* A TransactionEnvelope wraps a transaction with signatures. */
@@ -337,8 +354,7 @@ struct TransactionEnvelope
Transaction tx;
/* Each decorated signature is a signature over the SHA256 hash of
* a TransactionSignaturePayload */
- DecoratedSignature
- signatures<20>;
+ DecoratedSignature signatures<20>;
};
/* Operation Results section */
@@ -545,7 +561,8 @@ enum ChangeTrustResultCode
CHANGE_TRUST_NO_ISSUER = -2, // could not find issuer
CHANGE_TRUST_INVALID_LIMIT = -3, // cannot drop limit below balance
// cannot create with a limit of 0
- CHANGE_TRUST_LOW_RESERVE = -4, // not enough funds to create a new trust line,
+ CHANGE_TRUST_LOW_RESERVE =
+ -4, // not enough funds to create a new trust line,
CHANGE_TRUST_SELF_NOT_ALLOWED = -5 // trusting self is not allowed
};
@@ -568,7 +585,7 @@ enum AllowTrustResultCode
ALLOW_TRUST_NO_TRUST_LINE = -2, // trustor does not have a trustline
// source account does not require trust
ALLOW_TRUST_TRUST_NOT_REQUIRED = -3,
- ALLOW_TRUST_CANT_REVOKE = -4, // source account can't revoke trust,
+ ALLOW_TRUST_CANT_REVOKE = -4, // source account can't revoke trust,
ALLOW_TRUST_SELF_NOT_ALLOWED = -5 // trusting self is not allowed
};
@@ -587,10 +604,11 @@ enum AccountMergeResultCode
// codes considered as "success" for the operation
ACCOUNT_MERGE_SUCCESS = 0,
// codes considered as "failure" for the operation
- ACCOUNT_MERGE_MALFORMED = -1, // can't merge onto itself
- ACCOUNT_MERGE_NO_ACCOUNT = -2, // destination does not exist
- ACCOUNT_MERGE_IMMUTABLE_SET = -3, // source account has AUTH_IMMUTABLE set
- ACCOUNT_MERGE_HAS_SUB_ENTRIES = -4 // account has trust lines/offers
+ ACCOUNT_MERGE_MALFORMED = -1, // can't merge onto itself
+ ACCOUNT_MERGE_NO_ACCOUNT = -2, // destination does not exist
+ ACCOUNT_MERGE_IMMUTABLE_SET = -3, // source account has AUTH_IMMUTABLE set
+ ACCOUNT_MERGE_HAS_SUB_ENTRIES = -4, // account has trust lines/offers
+ ACCOUNT_MERGE_SEQNUM_TOO_FAR = -5 // sequence number is over max allowed
};
union AccountMergeResult switch (AccountMergeResultCode code)
@@ -632,10 +650,12 @@ enum ManageDataResultCode
// codes considered as "success" for the operation
MANAGE_DATA_SUCCESS = 0,
// codes considered as "failure" for the operation
- MANAGE_DATA_NOT_SUPPORTED_YET = -1, // The network hasn't moved to this protocol change yet
- MANAGE_DATA_NAME_NOT_FOUND = -2, // Trying to remove a Data Entry that isn't there
- MANAGE_DATA_LOW_RESERVE = -3, // not enough funds to create a new Data Entry
- MANAGE_DATA_INVALID_NAME = -4 // Name not a valid string
+ MANAGE_DATA_NOT_SUPPORTED_YET =
+ -1, // The network hasn't moved to this protocol change yet
+ MANAGE_DATA_NAME_NOT_FOUND =
+ -2, // Trying to remove a Data Entry that isn't there
+ MANAGE_DATA_LOW_RESERVE = -3, // not enough funds to create a new Data Entry
+ MANAGE_DATA_INVALID_NAME = -4 // Name not a valid string
};
union ManageDataResult switch (ManageDataResultCode code)
@@ -646,14 +666,32 @@ default:
void;
};
+/******* BumpSequence Result ********/
+
+enum BumpSequenceResultCode
+{
+ // codes considered as "success" for the operation
+ BUMP_SEQUENCE_SUCCESS = 0,
+ // codes considered as "failure" for the operation
+ BUMP_SEQUENCE_BAD_SEQ = -1 // `bumpTo` is not within bounds
+};
+
+union BumpSequenceResult switch (BumpSequenceResultCode code)
+{
+case BUMP_SEQUENCE_SUCCESS:
+ void;
+default:
+ void;
+};
/* High level Operation Result */
enum OperationResultCode
{
opINNER = 0, // inner object result is valid
- opBAD_AUTH = -1, // too few valid signatures / wrong network
- opNO_ACCOUNT = -2 // source account was not found
+ opBAD_AUTH = -1, // too few valid signatures / wrong network
+ opNO_ACCOUNT = -2, // source account was not found
+ opNOT_SUPPORTED = -3 // operation not supported at this time
};
union OperationResult switch (OperationResultCode code)
@@ -683,6 +721,8 @@ case opINNER:
InflationResult inflationResult;
case MANAGE_DATA:
ManageDataResult manageDataResult;
+ case BUMP_SEQUENCE:
+ BumpSequenceResult bumpSeqResult;
}
tr;
default: