From ebec83c70458acef756264aade1a2f596663383a Mon Sep 17 00:00:00 2001 From: Vitaliy Vlasov Date: Thu, 1 Mar 2018 17:52:17 +0200 Subject: [PATCH 1/4] [Fix #339] Update GitHub comment in deploy-pending-contracts scheduler thread; refactor contract deployment code to a separate fn; remove unused code --- resources/sql/queries.sql | 19 ++++------------- src/clj/commiteth/bounties.clj | 38 +++++++++++++++++---------------- src/clj/commiteth/db/issues.clj | 6 ------ src/clj/commiteth/scheduler.clj | 30 ++++++-------------------- 4 files changed, 30 insertions(+), 63 deletions(-) diff --git a/resources/sql/queries.sql b/resources/sql/queries.sql index 1421ced..2335e72 100644 --- a/resources/sql/queries.sql +++ b/resources/sql/queries.sql @@ -196,20 +196,6 @@ AND i.contract_address IS NULL AND i.transaction_hash IS NOT NULL; --- :name list-failed-deployments :? :* --- :doc retrieves failed contract deployments -SELECT - i.issue_id as issue_id, - i.transaction_hash as transaction_hash, - u.address as owner_address -FROM issues i, users u, repositories r -WHERE r.user_id = u.id -AND i.repo_id = r.repo_id -AND i.contract_address IS NULL -AND i.transaction_hash IS NOT NULL -AND i.updated < now() at time zone 'UTC' - interval '1 hour'; - - -- Pull Requests ------------------------------------------------------------------- -- :name save-pull-request! :! :n @@ -252,7 +238,10 @@ WHERE pr_id = :pr_id; -- :doc bounty issues where deploy contract has failed SELECT i.issue_id AS issue_id, - u.address AS owner_address + i.issue_number AS issue_number, + u.login AS owner, + u.address AS owner_address, + r.repo AS repo FROM issues i, users u, repositories r WHERE r.user_id = u.id diff --git a/src/clj/commiteth/bounties.clj b/src/clj/commiteth/bounties.clj index 19d995b..aa2e04c 100644 --- a/src/clj/commiteth/bounties.clj +++ b/src/clj/commiteth/bounties.clj @@ -20,6 +20,25 @@ (let [labels (:labels issue)] (some #(= label-name (:name %)) labels))) +(defn deploy-contract [owner owner-address repo issue-id issue-number] + (if (empty? owner-address) + (log/error "Unable to deploy bounty contract because" + "repo owner has no Ethereum addres") + (do + (log/info "deploying contract to " owner-address) + (if-let [transaction-hash (multisig/deploy-multisig owner-address)] + (do + (log/info "Contract deployed, transaction-hash:" + transaction-hash) + (->> (github/post-deploying-comment owner + repo + issue-number + transaction-hash) + :id + (issues/update-comment-id issue-id)) + (issues/update-transaction-hash issue-id transaction-hash)) + (log/error "Failed to deploy contract to" owner-address))))) + (defn add-bounty-for-issue [repo repo-id issue] (let [{issue-id :id issue-number :number @@ -29,24 +48,7 @@ owner :owner} (users/get-repo-owner repo-id)] (log/debug "Adding bounty for issue " repo issue-number "owner address: " owner-address) (if (= 1 created-issue) - (if (empty? owner-address) - (log/error "Unable to deploy bounty contract because" - "repo owner has no Ethereum addres") - (do - (log/debug "deploying contract to " owner-address) - (let [transaction-hash (multisig/deploy-multisig owner-address)] - (if (nil? transaction-hash) - (log/error "Failed to deploy contract to" owner-address) - (do - (log/info "Contract deployed, transaction-hash:" - transaction-hash) - (->> (github/post-deploying-comment owner - repo - issue-number - transaction-hash) - :id - (issues/update-comment-id issue-id)))) - (issues/update-transaction-hash issue-id transaction-hash)))) + (deploy-contract owner owner-address repo issue-id issue-number) (log/debug "Issue already exists in DB, ignoring")))) (defn maybe-add-bounty-for-issue [repo repo-id issue] diff --git a/src/clj/commiteth/db/issues.clj b/src/clj/commiteth/db/issues.clj index 834684f..37db8d1 100644 --- a/src/clj/commiteth/db/issues.clj +++ b/src/clj/commiteth/db/issues.clj @@ -63,12 +63,6 @@ (jdbc/with-db-connection [con-db *db*] (db/list-pending-deployments con-db))) - -(defn list-failed-deployments - [] - (jdbc/with-db-connection [con-db *db*] - (db/list-failed-deployments con-db))) - (defn update-eth-balance [contract-address balance-eth] (jdbc/with-db-connection [con-db *db*] diff --git a/src/clj/commiteth/scheduler.clj b/src/clj/commiteth/scheduler.clj index 0c35df3..f1535c7 100644 --- a/src/clj/commiteth/scheduler.clj +++ b/src/clj/commiteth/scheduler.clj @@ -54,33 +54,16 @@ (log/error "Failed to find contract address in tx logs"))))) -(defn deploy-contract [owner-address issue-id] - (let [transaction-hash (multisig/deploy-multisig owner-address)] - (if (nil? transaction-hash) - (log/error "Failed to deploy contract to" owner-address) - (log/info "Contract deployed, transaction-hash:" - transaction-hash )) - (issues/update-transaction-hash issue-id transaction-hash))) - - -(defn redeploy-failed-contracts - "If the bot account runs out of gas, we end up with transaction-id in db, but with nothing written to blockchain. In this case we should try to re-deploy the contract." - [] - (doseq [{issue-id :issue_id - transaction-hash :transaction_hash - owner-address :owner_address} (issues/list-failed-deployments)] - (when (nil? (eth/get-transaction-receipt transaction-hash)) - (log/info "Detected nil transaction receipt for pending contract deployment for issue" issue-id ", re-deploying contract") - (deploy-contract owner-address issue-id)))) - - (defn deploy-pending-contracts "Under high-concurrency circumstances or in case geth is in defunct state, a bounty contract may not deploy successfully when the bounty label is addded to an issue. This function deploys such contracts." [] (doseq [{issue-id :issue_id - owner-address :owner_address} (db-bounties/pending-contracts)] + issue-number :issue_number + owner :owner + owner-address :owner_address + repo :repo} (db-bounties/pending-contracts)] (log/debug "Trying to re-deploy failed bounty contract deployment, issue-id:" issue-id) - (deploy-contract owner-address issue-id))) + (bounties/deploy-contract owner owner-address repo issue-id issue-number))) (defn self-sign-bounty "Walks through all issues eligible for bounty payout and signs corresponding transaction" @@ -327,8 +310,7 @@ ;; TODO: disabled for now. looks like it may cause extraneus ;; contract deployments and costs (run-tasks - [;;redeploy-failed-contracts - deploy-pending-contracts + [deploy-pending-contracts update-issue-contract-address update-confirm-hash update-payout-receipt From 8c44e101287df39c913df916acde098ccf9c8651 Mon Sep 17 00:00:00 2001 From: Vitaliy Vlasov Date: Fri, 16 Mar 2018 17:40:31 +0200 Subject: [PATCH 2/4] Add some logging for GitHub comment posting --- src/clj/commiteth/bounties.clj | 13 +++++++------ src/clj/commiteth/github/core.clj | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/clj/commiteth/bounties.clj b/src/clj/commiteth/bounties.clj index aa2e04c..d911e6c 100644 --- a/src/clj/commiteth/bounties.clj +++ b/src/clj/commiteth/bounties.clj @@ -30,12 +30,13 @@ (do (log/info "Contract deployed, transaction-hash:" transaction-hash) - (->> (github/post-deploying-comment owner - repo - issue-number - transaction-hash) - :id - (issues/update-comment-id issue-id)) + (let [resp (github/post-deploying-comment owner + repo + issue-number + transaction-hash) + _ (log/info "post-deploying-comment response:" resp) + comment-id (:id resp)] + (issues/update-comment-id issue-id comment-id)) (issues/update-transaction-hash issue-id transaction-hash)) (log/error "Failed to deploy contract to" owner-address))))) diff --git a/src/clj/commiteth/github/core.clj b/src/clj/commiteth/github/core.clj index f6b4ea9..fafb577 100644 --- a/src/clj/commiteth/github/core.clj +++ b/src/clj/commiteth/github/core.clj @@ -272,7 +272,7 @@ (defn post-deploying-comment [owner repo issue-number tx-id] (let [comment (generate-deploying-comment owner repo issue-number tx-id)] - (log/debug "Posting comment to" (str owner "/" repo "/" issue-number) ":" comment) + (log/info "Posting comment to" (str owner "/" repo "/" issue-number) ":" comment) (issues/create-comment owner repo issue-number comment (self-auth-params)))) (defn make-patch-request [end-point positional query] From b833cee66dfbd98b46d2032abba41256202658d2 Mon Sep 17 00:00:00 2001 From: Martin Klepsch Date: Tue, 20 Mar 2018 12:46:14 +0100 Subject: [PATCH 3/4] throw if eth-rpc returns error --- src/clj/commiteth/eth/core.clj | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/clj/commiteth/eth/core.clj b/src/clj/commiteth/eth/core.clj index cd9ecfc..8f45808 100644 --- a/src/clj/commiteth/eth/core.clj +++ b/src/clj/commiteth/eth/core.clj @@ -127,13 +127,17 @@ result (safe-read-str (:body response))] (log/debug body "\n" result) - (if (= (:id result) request-id) - (:result result) - (do - (log/error "Geth returned an invalid json-rpc request ID," - "ignoring response") - (when-let [error (:error result)] - (log/error "Method: " method ", error: " error)))))) + (cond + ;; Ignore any responses that have mismatching request ID + (not= (:id result) request-id) + (log/error "Geth returned an invalid json-rpc request ID, ignoring response") + + ;; If request ID matches but contains error, throw + (:error result) + (throw (ex-info "Error submitting transaction via eth-rpc" (:error result))) + + :else + (:result result)))) (defn hex->big-integer [hex] From 4734a2e82ec2e5d80d48435f5f20cc1bbc4dc024 Mon Sep 17 00:00:00 2001 From: Vitaliy Vlasov Date: Thu, 22 Mar 2018 13:17:30 +0200 Subject: [PATCH 4/4] Fix pending-contracts query to return proper repo owner --- resources/sql/queries.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/sql/queries.sql b/resources/sql/queries.sql index dbe24c3..27f5d68 100644 --- a/resources/sql/queries.sql +++ b/resources/sql/queries.sql @@ -236,7 +236,7 @@ WHERE pr_id = :pr_id; SELECT i.issue_id AS issue_id, i.issue_number AS issue_number, - u.login AS owner, + r.owner AS owner, u.address AS owner_address, r.repo AS repo FROM issues i, users u, repositories r