fix #399: track each internal transaction and associated nonce

src/clj/commiteth/bounties.clj

@@ -25,7 +25,8 @@
     (log/errorf "issue %s: Unable to deploy bounty contract because repo owner has no Ethereum addres" issue-id)
     (try
       (log/infof "issue %s: Deploying contract to %s" issue-id owner-address)
-      (if-let [transaction-hash (multisig/deploy-multisig owner-address)]
+      (if-let [transaction-hash (multisig/deploy-multisig {:owner owner-address
+                                                           :internal-tx-id (str "contract-github-issue-" issue-id)})]
         (do
           (log/infof "issue %s: Contract deployed, transaction-hash: %s" issue-id transaction-hash)
           (let [resp (github/post-deploying-comment owner

src/clj/commiteth/eth/core.clj

@@ -28,7 +28,9 @@
 (defn auto-gas-price? [] (env :auto-gas-price false))
 (defn offline-signing? [] (env :offline-signing true))
 
-(def web3j-obj (atom nil))
+(def web3j-obj
+  (delay (Web3j/build (HttpService. (eth-rpc-url)))))
+
 (def creds-obj (atom nil))
 
 (defn wallet-file-path []
@@ -37,10 +39,6 @@
 (defn wallet-password []
   (env :eth-password))
 
-(defn create-web3j []
- (or @web3j-obj
-      (swap! web3j-obj (constantly (Web3j/build (HttpService. (eth-rpc-url)))))))
-
 (defn creds []
   (or @creds-obj
       (let [password  (wallet-password)
@@ -53,37 +51,48 @@
           (throw (ex-info "Make sure you provided proper credentials in appropriate resources/config.edn"
                           {:password password :file-path file-path}))))))
 
-(defn get-nonce []
+(defprotocol INonceTracker
-  (let [current-nonce (atom nil)]
+  "The reason we need this is that we might send mutliple identical
-    (fn []
+  transactions (e.g. bounty contract deploy with same owner) shortly
-      (let [nonce (.. (.ethGetTransactionCount (create-web3j) 
+  after another. In this case web3j's TX counting only increases once
-                                               (env :eth-account) 
+  a transaction has been confirmed and so we send multiple identical
-                                               DefaultBlockParameterName/LATEST)
+  transactions with the same nonce."
-                      sendAsync
+  (get-nonce [this internal-tx-id]
-                      get
+    "Return the to be used nonce for an OpenBounty Ethereum
-                      getTransactionCount)]
+    transaction identified by `internal-tx-id`. As these IDs are stable
-        (if (= nonce @current-nonce)
+    we can use them to use consistent nonces for the same transaction."))
-          (throw (Exception. (str "Attempting to create transaction with the same nonce: " nonce)))
-          (swap! current-nonce (constantly nonce)))
-        nonce))))
-(def get-nonce-fn (get-nonce))
 
-(defn get-signed-tx [gas-price gas-limit to data]
+(defrecord NonceTracker [state]
-  "Create a sign a raw transaction.
+  INonceTracker
-   'From' argument is not needed as it's already
+  (get-nonce [this internal-tx-id]
-   encoded in credentials.
+    (or (get @state internal-tx-id)
+        (let [nonces         (set (vals @state))
+              web3j-tx-count (.. (.ethGetTransactionCount
+                                  @web3j-obj
+                                  (env :eth-account)
+                                  DefaultBlockParameterName/LATEST)
+                                 sendAsync
+                                 get
+                                 getTransactionCount)]
+          ;; TODO this is a memory leak since tracking state is never pruned
+          ;; Since we're not doing 1000s of transactions every day yet we can
+          ;; probably defer worrying about this until a bit later
+          (if (contains? nonces web3j-tx-count)
+            (swap! state assoc internal-tx-id (inc (max nonces)))
+            (swap! state assoc internal-tx-id web3j-tx-count))))))
+
+(def nonce-tracker
+  (->NonceTracker (atom {})))
+
+(defn get-signed-tx [{:keys [gas-price gas-limit to data internal-tx-id]}]
+  "Create a sign a raw transaction. 'From' argument is not needed as it's already encoded in credentials.
    See https://web3j.readthedocs.io/en/latest/transactions.html#offline-transaction-signing"
-  (let [nonce (get-nonce-fn)
+  (let [nonce (get-nonce nonce-tracker internal-tx-id)
-        tx (RawTransaction/createTransaction
+        tx (RawTransaction/createTransaction nonce gas-price gas-limit to data)
-             nonce
-             gas-price
-             gas-limit
-             to
-             data)
         signed (TransactionEncoder/signMessage tx (creds))
         hex-string (Numeric/toHexString signed)]
-    (log/infof "Signing TX: nonce: %s, gas-price: %s, gas-limit: %s, data: %s"
+    (log/infof "Signing TX %s: nonce: %s, gas-price: %s, gas-limit: %s, data: %s"
-               nonce gas-price gas-limit data)
+               internal-tx-id nonce gas-price gas-limit data)
     hex-string))
 
 (defn eth-gasstation-gas-price
@@ -253,7 +262,8 @@
     (eth-rpc "eth_call" [{:to contract :data data} "latest"])))
 
 (defn execute
-  [from contract method-id gas-limit & params]
+  [{:keys [from contract method-id gas-limit params internal-tx-id]}]
+  {:pre [(string? method-id)]}
   (let [data (apply format-call-params method-id params)
         gas-price (gas-price)
         value (format "0x%x" 0)
@@ -264,13 +274,13 @@
                  (merge {:gasPrice (integer->hex gas-price)})
                  contract
                  (merge {:to contract}))
-        gas (if gas-limit gas-limit 
+        gas    (or gas-limit (estimate-gas from contract value params))
-              (estimate-gas from contract value params))
         params (if (offline-signing?)
-                 (get-signed-tx (biginteger gas-price)
+                 (get-signed-tx {:internal-tx-id internal-tx-id
-                                      (hex->big-integer gas)
+                                 :gas-price (biginteger gas-price)
-                                      contract
+                                 :gas-limit (hex->big-integer gas)
-                                      data) 
+                                 :to        contract
+                                 :data      data})
                  (assoc params :gas gas))]
     (if (offline-signing?)
       (eth-rpc

src/clj/commiteth/eth/multisig_wallet.clj

@@ -1,6 +1,5 @@
 (ns commiteth.eth.multisig-wallet
-  (:require [commiteth.eth.core :as eth
+  (:require [commiteth.eth.core :as eth]
-             :refer [create-web3j creds]]
             [commiteth.config :refer [env]]
             [clojure.tools.logging :as log]
             [taoensso.tufte :as tufte :refer (defnp p profiled profile)]
@@ -36,24 +35,19 @@
   []
   (env :tokenreg-base-format :status))
 
-(defn create-new
-  [owner1 owner2 required]
-  (eth/execute (eth/eth-account)
-               (factory-contract-addr)
-               (:create method-ids)
-               (eth/integer->hex 865000) ;; gas-limit
-               0x40
-               0x2
-               required
-               owner1
-               owner2))
-
-
 (defn deploy-multisig
   "Deploy a new multisig contract to the blockchain with commiteth bot
-  and given owner as owners."
+  and given owner as owners.
-  [owner]
+  `internal-tx-id` is used to identify what issue this multisig is deployed
-  (create-new (eth/eth-account) owner 2))
+  for and manage nonces at a later point in time."
+  [{:keys [owner internal-tx-id]}]
+  {:pre [(string? owner) (string? internal-tx-id)]}
+  (eth/execute {:internal-tx-id internal-tx-id
+                :from      (eth/eth-account)
+                :contract  (factory-contract-addr)
+                :method-id (:create method-ids)
+                :gas-limit (eth/integer->hex 865000)
+                :params    [0x40 0x2 2 (eth/eth-account) owner]}))
 
 (defn find-event-in-tx-receipt [tx-receipt topic-id]
   (let [logs (:logs tx-receipt)
@@ -92,20 +86,18 @@
 
 
 (defn send-all
-  [contract to]
+  [{:keys [contract payout-address internal-tx-id]}]
-  (log/debug "multisig/send-all " contract to)
+  {:pre [(string? contract) (string? payout-address) (string? internal-tx-id)]}
+  (log/debug "multisig/send-all " contract payout-address internal-tx-id)
   (let [params (eth/format-call-params
                 (:withdraw-everything method-ids)
-                to)]
+                payout-address)]
-    (eth/execute (eth/eth-account)
+    (eth/execute {:internal-tx-id internal-tx-id
-                 contract
+                  :from      (eth/eth-account)
-                 (:submit-transaction method-ids)
+                  :contract  contract
-                 nil
+                  :method-id (:submit-transaction method-ids)
-                 contract
+                  :gas-limit nil
-                 0
+                  :params    [contract 0 "0x60" "0x24" params]})))
-                 "0x60" ;; TODO: document these
-                 "0x24" ;;  or refactor out
-                 params)))
 
 
 (defn get-token-address [token]
@@ -118,17 +110,16 @@
   (log/debug "multisig/watch-token" bounty-addr token)
   (let [token-address (get-token-address token)]
     (assert token-address)
-    (eth/execute (eth/eth-account)
+    (eth/execute {:from      (eth/eth-account)
-                 bounty-addr
+                  :contract  bounty-addr
-                 (:watch method-ids)
+                  :method-id (:watch method-ids)
-                 nil
+                  :gas-limit nil
-                 token-address)))
+                  :params    [token-address]})))
-
 
 (defn load-bounty-contract [addr]
   (MultiSigTokenWallet/load addr
-    (create-web3j)
+    @eth/web3j-obj
-    (creds)
+    (eth/creds)
     (eth/gas-price)
     (BigInteger/valueOf 500000)))
 

src/clj/commiteth/eth/token_registry.clj

@@ -1,6 +1,5 @@
 (ns commiteth.eth.token-registry
-  (:require [commiteth.eth.core :as eth
+  (:require [commiteth.eth.core :as eth]
-             :refer [create-web3j creds]]
             [commiteth.config :refer [env]]
             [clojure.tools.logging :as log])
   (:import [org.web3j
@@ -23,8 +22,8 @@
 
 (defn- load-tokenreg-contract [addr]
   (TokenReg/load addr
-                 (create-web3j)
+                 @eth/web3j-obj
-                 (creds)
+                 (eth/creds)
                  (eth/gas-price)
                  (BigInteger/valueOf 21000)))
 
@@ -59,9 +58,8 @@
 (defn deploy-parity-tokenreg
   "Deploy an instance of parity token-registry to current network"
   []
-  (let [web3j (create-web3j)]
+  (TokenReg/deploy @eth/web3j-obj
-    (TokenReg/deploy web3j
+                   (eth/creds)
-                     (creds)
+                   (eth/gas-price)
-                     (eth/gas-price)
+                   (BigInteger/valueOf 4000000) ;; gas limit
-                     (BigInteger/valueOf 4000000) ;; gas limit
+                   BigInteger/ZERO))
-                     BigInteger/ZERO)))

src/clj/commiteth/scheduler.clj

@@ -126,7 +126,9 @@
                                                    tokens
                                                    winner-login
                                                    true))
-             (let [execute-hash (multisig/send-all contract-address payout-address)]
+             (let [execute-hash (multisig/send-all {:contract contract-address
+                                                    :payout-address payout-address
+                                                    :internal-tx-id (str "payout-github-issue-" issue-id)})]
                (log/infof "issue %s: Payout self-signed, called sign-all(%s) tx: %s" issue-id contract-address payout-address execute-hash)
                (db-bounties/update-execute-hash issue-id execute-hash)
                (db-bounties/update-winner-login issue-id winner-login)