status-im / open-bounty

Enable communities to distribute funds to push their cause forward.
https://openbounty.status.im/
GNU Affero General Public License v3.0
118 stars 36 forks source link

Send txs sequentially; add thread to check for unmined txs #427

Closed vitvly closed 6 years ago

vitvly commented 6 years ago

This PR does the following:

Some additional refactors have been made:

After QA:

Small issues to fix:

vitvly commented 6 years ago

Moved web3j-related code and tracker-related code to separate namespaces.

vitvly commented 6 years ago

QA notes:

As expired deployments/submissions/watches are re-executed now, it is necessary to check whether they actually do:)

Queries below will simulate a situation where a tx hash for deployment/execution haven't been mined in 15 minutes, therefore making it a candidate for re-execution.

  1. In order to check whether expired deployments are re-executed:
    update issues set transaction_hash='<some_nonexisting_hash>', contract_address=null, updated=updated - interval '15 minutes' where issue_id=<issue_id>;
  2. In order to check whether expired submissions (once PR has been merged) are re-executed:
    update issues set execute_hash='<some_nonexisting_hash>', confirm_hash=null, updated=updated - interval '15 minutes' where issue_id=<issue_id>;
churik commented 6 years ago

1. Too small timeout which triggers transaction to re-execute (Mainnet)

Steps: Requirements: GH account is whitelisted, signed app, test application is added to repo;

2. Two comments are persisted if 1 transaction was canceled because of timeout

Steps: Requirements: GH account is whitelisted, signed app, test application is added to repo;

3. 'Deploying contract' - usually takes 1-2 min - is too optimistic

I suggest to replace it with 3- to timeout time(IMO 10 min) d

churik commented 6 years ago

Tested functionality

Tested on Ropsten and Mainnet.

Comment 3. 'Deploying contract' - usually takes 1-2 min - is too optimistic depends on network speed and should be checked on prod and discussed separately, probably will move to the separate issue.

churik commented 6 years ago

@martinklepsch please merge if it is OK after review; QA is passed.