diff --git a/src/clj/commiteth/bounties.clj b/src/clj/commiteth/bounties.clj index d1ee19d..a2c93c6 100644 --- a/src/clj/commiteth/bounties.clj +++ b/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 diff --git a/src/clj/commiteth/eth/core.clj b/src/clj/commiteth/eth/core.clj index c85df48..93c9352 100644 --- a/src/clj/commiteth/eth/core.clj +++ b/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,38 +51,59 @@ (throw (ex-info "Make sure you provided proper credentials in appropriate resources/config.edn" {:password password :file-path file-path})))))) -(defn get-nonce [] - (let [current-nonce (atom nil)] - (fn [] - (let [nonce (.. (.ethGetTransactionCount (create-web3j) - (env :eth-account) - DefaultBlockParameterName/LATEST) - sendAsync - get - getTransactionCount)] - (if (= nonce @current-nonce) - (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-web3j-nonce [web3j-instance] + (.. (.ethGetTransactionCount web3j-instance (env :eth-account) DefaultBlockParameterName/LATEST) + sendAsync + get + getTransactionCount)) -(defn get-signed-tx [gas-price gas-limit to data] - "Create a sign a raw transaction. - 'From' argument is not needed as it's already - encoded in credentials. +(defprotocol INonceTracker + "The reason we need this is that we might send mutliple identical + transactions (e.g. bounty contract deploy with same owner) shortly + after another. In this case web3j's TX counting only increases once + a transaction has been confirmed and so we send multiple identical + transactions with the same nonce. + + Web3j also provides a TransactionManager that increases nonces but it + does not track nonces related to transactions and so as far as I understand + it might cause transactions to be executed twice if they are retried. + + https://github.com/web3j/web3j/blob/d19855475aa6620a7e93523bd9ede26ca50ed042/core/src/main/java/org/web3j/tx/RawTransactionManager.java" + (get-nonce [this internal-tx-id] + "Return the to be used nonce for an OpenBounty Ethereum + transaction identified by `internal-tx-id`. As these IDs are stable + we can use them to use consistent nonces for the same transaction.")) + +(defrecord NonceTracker [state] + INonceTracker + (get-nonce [this internal-tx-id] + (let [prev-nonce (get @state internal-tx-id) + web3j-nonce (get-web3j-nonce @web3j-obj) + nonces (set (vals @state)) + nonce (if (seq nonces) + (inc (apply max nonces)) + web3j-nonce)] + (when prev-nonce + (log/warnf "%s: tx will be retried (prev-nonce: %s, new-nonce: %s, web3j-nonce: %s)" + internal-tx-id prev-nonce nonce web3j-nonce)) + ;; 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 + (swap! state assoc internal-tx-id nonce) + nonce))) + +(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) - tx (RawTransaction/createTransaction - 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" - nonce gas-price gas-limit data) - hex-string)) + (let [nonce (get-nonce nonce-tracker internal-tx-id)] + (log/infof "%s: Signing nonce: %s, gas-price: %s, gas-limit: %s" + internal-tx-id nonce gas-price gas-limit) + (-> (RawTransaction/createTransaction (biginteger nonce) gas-price gas-limit to data) + (TransactionEncoder/signMessage (creds)) + (Numeric/toHexString)))) (defn eth-gasstation-gas-price "Get max of average and average_calc from gas station and use that @@ -139,7 +158,8 @@ (atom 0)) (defn eth-rpc - [method params] + [{:keys [method params internal-tx-id]}] + {:pre [(string? method) (some? params)]} (let [request-id (swap! req-id-tracker inc) body {:jsonrpc "2.0" :method method @@ -149,9 +169,10 @@ :body (json/write-str body)} response @(post (eth-rpc-url) options) result (safe-read-str (:body response))] - (log/infof "eth-rpc req(%s) body: %s\neth-rpc req(%s) result: %s" - request-id body request-id result) - + (when internal-tx-id + (log/infof "%s: eth-rpc %s" internal-tx-id method)) + (log/debugf "%s: eth-rpc req(%s) body: %s" internal-tx-id request-id body) + (log/debugf "%s: eth-rpc req(%s) result: %s" internal-tx-id request-id result) (cond ;; Ignore any responses that have mismatching request ID (not= (:id result) request-id) @@ -159,7 +180,10 @@ ;; If request ID matches but contains error, throw (:error result) - (throw (ex-info "Error submitting transaction via eth-rpc" (:error result))) + (throw + (ex-info (format "%s: Error submitting transaction via eth-rpc %s" + (or internal-tx-id "(no-tx-id)") (:error result)) + (:error result))) :else (:result result)))) @@ -196,9 +220,10 @@ (defn estimate-gas [from to value & [params]] (let [geth-estimate (eth-rpc - "eth_estimateGas" [(merge params {:from from - :to to - :value value})]) + {:method "eth_estimateGas" + :params [(merge params {:from from + :to to + :value value})]}) adjusted-gas (adjust-gas-estimate geth-estimate)] (log/debug "estimated gas (geth):" geth-estimate) @@ -221,7 +246,8 @@ (defn get-balance-hex [account] - (eth-rpc "eth_getBalance" [account "latest"])) + (eth-rpc {:method "eth_getBalance" + :params [account "latest"]})) (defn get-balance-wei [account] @@ -240,7 +266,8 @@ (defn get-transaction-receipt [hash] - (eth-rpc "eth_getTransactionReceipt" [hash])) + (eth-rpc {:method "eth_getTransactionReceipt" + :params [hash]})) (defn format-call-params [method-id & params] @@ -250,10 +277,12 @@ (defn call [contract method-id & params] (let [data (apply format-call-params method-id params)] - (eth-rpc "eth_call" [{:to contract :data data} "latest"]))) + (eth-rpc {:method "eth_call" + :params [{: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,21 +293,23 @@ (merge {:gasPrice (integer->hex gas-price)}) contract (merge {:to contract})) - gas (if gas-limit gas-limit - (estimate-gas from contract value params)) + gas (or gas-limit (estimate-gas from contract value params)) params (if (offline-signing?) - (get-signed-tx (biginteger gas-price) - (hex->big-integer gas) - contract - data) + (get-signed-tx {:internal-tx-id internal-tx-id + :gas-price (biginteger gas-price) + :gas-limit (hex->big-integer gas) + :to contract + :data data}) (assoc params :gas gas))] (if (offline-signing?) (eth-rpc - "eth_sendRawTransaction" - [params]) + {:method "eth_sendRawTransaction" + :params [params] + :internal-tx-id internal-tx-id}) (eth-rpc - "personal_sendTransaction" - [params (eth-password)])))) + {:method "personal_sendTransaction" + :params [params (eth-password)] + :internal-tx-id internal-tx-id})))) (defn hex-ch->num [ch] @@ -339,7 +370,6 @@ eth-core :start (do - (swap! web3j-obj (constantly nil)) (swap! creds-obj (constantly nil)) (log/info "eth/core started")) :stop diff --git a/src/clj/commiteth/eth/multisig_wallet.clj b/src/clj/commiteth/eth/multisig_wallet.clj index 941b0ca..874c113 100644 --- a/src/clj/commiteth/eth/multisig_wallet.clj +++ b/src/clj/commiteth/eth/multisig_wallet.clj @@ -1,6 +1,5 @@ (ns commiteth.eth.multisig-wallet - (:require [commiteth.eth.core :as eth - :refer [create-web3j creds]] + (:require [commiteth.eth.core :as eth] [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." - [owner] - (create-new (eth/eth-account) owner 2)) + and given owner as owners. + `internal-tx-id` is used to identify what issue this multisig is deployed + 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] - (log/debug "multisig/send-all " contract to) + [{:keys [contract payout-address internal-tx-id]}] + {: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)] - (eth/execute (eth/eth-account) - contract - (:submit-transaction method-ids) - nil - contract - 0 - "0x60" ;; TODO: document these - "0x24" ;; or refactor out - params))) + payout-address)] + (eth/execute {:internal-tx-id internal-tx-id + :from (eth/eth-account) + :contract contract + :method-id (:submit-transaction method-ids) + :gas-limit nil + :params [contract 0 "0x60" "0x24" params]}))) (defn get-token-address [token] @@ -118,17 +110,17 @@ (log/debug "multisig/watch-token" bounty-addr token) (let [token-address (get-token-address token)] (assert token-address) - (eth/execute (eth/eth-account) - bounty-addr - (:watch method-ids) - nil - token-address))) - + (eth/execute {:internal-tx-id (str "watch-token-" (System/currentTimeMillis) "-" bounty-addr) + :from (eth/eth-account) + :contract bounty-addr + :method-id (:watch method-ids) + :gas-limit nil + :params [token-address]}))) (defn load-bounty-contract [addr] (MultiSigTokenWallet/load addr - (create-web3j) - (creds) + @eth/web3j-obj + (eth/creds) (eth/gas-price) (BigInteger/valueOf 500000))) diff --git a/src/clj/commiteth/eth/token_registry.clj b/src/clj/commiteth/eth/token_registry.clj index 4d1ddea..c7aeec7 100644 --- a/src/clj/commiteth/eth/token_registry.clj +++ b/src/clj/commiteth/eth/token_registry.clj @@ -1,6 +1,5 @@ (ns commiteth.eth.token-registry - (:require [commiteth.eth.core :as eth - :refer [create-web3j creds]] + (:require [commiteth.eth.core :as eth] [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) - (creds) + @eth/web3j-obj + (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 web3j - (creds) - (eth/gas-price) - (BigInteger/valueOf 4000000) ;; gas limit - BigInteger/ZERO))) + (TokenReg/deploy @eth/web3j-obj + (eth/creds) + (eth/gas-price) + (BigInteger/valueOf 4000000) ;; gas limit + BigInteger/ZERO)) diff --git a/src/clj/commiteth/scheduler.clj b/src/clj/commiteth/scheduler.clj index 1be116c..4299c49 100644 --- a/src/clj/commiteth/scheduler.clj +++ b/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)