From 80e535a13c8d988fd8fa438e7ea038540a6c0fb5 Mon Sep 17 00:00:00 2001
From: seelabs <scott.determan@yahoo.com>
Date: Sat, 23 Mar 2019 14:01:06 -0400
Subject: [PATCH] Arguments passed to jtx Env::operator() must be invocable:

Before this patch, jtx allowed non-invocable functions to be passed to
operator(). However, these arguments are ignored. This caused erronious code
code such as:

```
env (offer (account_to_test, BTC (250), XRP (1000)),
         offers (account_to_test, 1));
```

While it looks like the number of offers are checked, they are not. The `offers`
funclet is never run. While we could modify jtx to make the above code correct,
a cleaner solution is to run post conditions in a `require` statement after a
transasction runs.
---
 src/test/app/Offer_test.cpp | 18 +++++-----
 src/test/jtx/Env.h          | 68 ++++++-------------------------------
 2 files changed, 19 insertions(+), 67 deletions(-)

diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp
index bd0ece9d4..60ba66350 100644
--- a/src/test/app/Offer_test.cpp
+++ b/src/test/app/Offer_test.cpp
@@ -1126,15 +1126,15 @@ public:
         // PART 1:
         // we will make two offers that can be used to bridge BTC to USD
         // through XRP
-        env (offer (account_to_test, BTC (250), XRP (1000)),
-                 offers (account_to_test, 1));
+        env (offer (account_to_test, BTC (250), XRP (1000)));
+        env.require (offers(account_to_test, 1));
 
         // validate that the book now shows a BTC for XRP offer
         BEAST_EXPECT(isOffer(env, account_to_test, BTC(250), XRP(1000)));
 
         auto const secondLegSeq = env.seq(account_to_test);
-        env (offer (account_to_test, XRP(1000), USD (50)),
-                 offers (account_to_test, 2));
+        env (offer (account_to_test, XRP(1000), USD (50)));
+        env.require (offers (account_to_test, 2));
 
         // validate that the book also shows a XRP for USD offer
         BEAST_EXPECT(isOffer(env, account_to_test, XRP(1000), USD(50)));
@@ -1187,8 +1187,8 @@ public:
         // PART 2:
         // simple direct crossing  BTC to USD and then USD to BTC which causes
         // the first offer to be replaced
-        env (offer (account_to_test, BTC (250), USD (50)),
-                 offers (account_to_test, 1));
+        env (offer (account_to_test, BTC (250), USD (50)));
+        env.require (offers (account_to_test, 1));
 
         // validate that the book shows one BTC for USD offer and no USD for
         // BTC offers
@@ -1200,8 +1200,8 @@ public:
 
         // this second offer would self-cross directly, so it causes the first
         // offer by the same owner/taker to be removed
-        env (offer (account_to_test, USD (50), BTC (250)),
-                 offers (account_to_test, 1));
+        env (offer (account_to_test, USD (50), BTC (250)));
+        env.require (offers (account_to_test, 1));
 
         // validate that we now have just the second offer...the first
         // was removed
@@ -4586,7 +4586,7 @@ public:
             env(txn, ter (temBAD_TICK_SIZE));
 
             txn[sfTickSize.fieldName] = 0;
-            env(txn, tesSUCCESS);
+            env(txn);
             BEAST_EXPECT (! env.le(gw)->isFieldPresent (sfTickSize));
         }
 
diff --git a/src/test/jtx/Env.h b/src/test/jtx/Env.h
index 8ac47c160..9fe44862f 100644
--- a/src/test/jtx/Env.h
+++ b/src/test/jtx/Env.h
@@ -667,73 +667,25 @@ protected:
     std::shared_ptr<STTx const>
     st (JTx const& jt);
 
-    inline
-    void
-    invoke (STTx& stx)
-    {
-    }
-
-    template <class F>
-    inline
-    void
-    maybe_invoke (STTx& stx, F const& f,
-        std::false_type)
-    {
-    }
-
-    template <class F>
-    void
-    maybe_invoke (STTx& stx, F const& f,
-        std::true_type)
-    {
-        f(*this, stx);
-    }
-
     // Invoke funclets on stx
     // Note: The STTx may not be modified
-    template <class F, class... FN>
+    template <class... FN>
     void
-    invoke (STTx& stx, F const& f,
-        FN const&... fN)
+    invoke (STTx& stx, FN const&... fN)
     {
-        maybe_invoke(stx, f,
-            std::is_invocable<F,
-                void(Env&, STTx const&)>());
-        invoke(stx, fN...);
-    }
-
-    inline
-    void
-    invoke (JTx&)
-    {
-    }
-
-    template <class F>
-    inline
-    void
-    maybe_invoke (JTx& jt, F const& f,
-        std::false_type)
-    {
-    }
-
-    template <class F>
-    void
-    maybe_invoke (JTx& jt, F const& f,
-        std::true_type)
-    {
-        f(*this, jt);
+        // Sean Parent for_each_argument trick (C++ fold expressions would be
+        // nice here)
+        (void)std::array<int, sizeof...(fN)>{{((fN(*this, stx)), 0)...}};
     }
 
     // Invoke funclets on jt
-    template <class F, class... FN>
+    template <class... FN>
     void
-    invoke (JTx& jt, F const& f,
-        FN const&... fN)
+    invoke (JTx& jt, FN const&... fN)
     {
-        maybe_invoke(jt, f,
-            std::is_invocable<F,
-                void(Env&, JTx&)>());
-        invoke(jt, fN...);
+        // Sean Parent for_each_argument trick (C++ fold expressions would be
+        // nice here)
+        (void)std::array<int, sizeof...(fN)>{{((fN(*this, jt)), 0)...}};
     }
 
     // Map of account IDs to Account