Add reorged docker compose #1579

ayrat555 · 2020-06-13T13:10:58Z

Overview

Adds a step to circle ci that runs cabbage tests using 2 node ethash geth that can be used to trigger reorgs

docker-compose-reorg.yml

priv/cabbage/apps/itest/lib/reorg.ex

ayrat555 · 2020-06-18T13:34:17Z

It seems not all tests can survive reorgs

unnawut

VERY COOL!

priv/cabbage/apps/itest/lib/account.ex

priv/cabbage/apps/itest/test/itest/configuration_api_test.exs

priv/cabbage/apps/itest/test/itest/deposits_test.exs

InoMurko · 2020-06-22T11:09:31Z

priv/cabbage/apps/itest/test/itest/in_flight_exits_test.exs

@@ -62,6 +63,7 @@ defmodule InFlightExitsTests do
  @gas_process_exit_price 1_000_000_000

  setup do
+    Reorg.finish_reorg()
    # as we're testing IFEs, queue needs to be empty
    0 = get_next_exit_from_queue()


I think this is still a problem with eth hash nodes returning revert reasons rather then 0 right?

I added a hack for it 1ef4b25

@InoMurko It seems event with this hack, in-flight tests still fail

** (MatchError) no match of right hand side value: {:error, %{"code" => 3, "data" => "0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000004050696767796261636b20697320706f737369626c65206f6e6c7920696e20746865206669727374207068617365206f6620746865206578697420706572696f64", "message" => "execution reverted: Piggyback is possible only in the first phase of the exit period"}}

I think reorgs leave these test in inconsistent state

ayrat555 · 2020-07-04T19:45:20Z

@InoMurko I finally made test failures consistent by waiting for nodes to reconnect to each by calling net_peerCount. 32e2394
Is this last step? PR's good to merge?

.circleci/config.yml

InoMurko · 2020-07-06T08:54:34Z

.circleci/config.yml

+      - run:
+          name: (Perf) Format generated code and check for warnings
+          command: |
+            cd priv/perf
+            # run format ONLY on formatted code so that it cleans up quoted atoms because
+            # we cannot exclude folders to --warnings-as-errors
+            mix format apps/*_api/lib/*_api/model/*.ex
+            make format-code-check-warnings


.circleci/config.yml

InoMurko · 2020-07-06T08:59:56Z

priv/cabbage/apps/itest/lib/client.ex

+    case Jason.decode!(response.body)["data"] do
+      %{
+        "result" => "complete",
+        "transactions" => [
+          %{
+            "sign_hash" => sign_hash,
+            "typed_data" => typed_data,
+            "txbytes" => txbytes
+          }
+        ]
+      } ->
+        {:ok, [sign_hash, typed_data, txbytes]}
+
+      %{"code" => "create:client_error", "messages" => %{"code" => "operation:service_unavailable"}} = result ->
+        if tries == 0 do
+          result
+        else
+          Process.sleep(1_000)
+          create_transaction(amount_in_wei, input_address, output_address, currency, tries - 1)
+        end
+    end


perhaps we should break this into functions.

result = Jason.decode!(response.body)["data"]

case process_transaction_result() do {:ok, [sign_hash, typed_data, txbytes]} -> {:ok, [sign_hash, typed_data, txbytes]} :unavailable -> if tries == 0 do result else Process.sleep(1_000) create_transaction(amount_in_wei, input_address, output_address, currency, tries - 1) end end

why does operation:service_unavailable happen?

it's happening during reorgs. when one node is paused. I think it happens because Nginx continues to redirect to paused node

aha, so perhaps an alarm is raised (ethereum connection error?)

get-alarms: echo "Child Chain alarms" ; \ curl -s -X GET http://localhost:9656/alarm.get ; \ echo "\nWatcher alarms" ; \ curl -s -X GET http://localhost:${WATCHER_PORT}/alarm.get ; \ echo "\nWatcherInfo alarms" ; \ curl -s -X GET http://localhost:${WATCHER_INFO_PORT}/alarm.get

I can't see any alarms. but in Nginx logs I see

nginx | 2020/07/06 11:36:53 [error] 29#29: *37837 upstream timed out (110: Connection timed out) while reading response header from upstream, client: 172.25.0.105, server: , request: "POST / HTTP/1.1", upstream: "http://172.25.0.103:8545/", host: "172.25.0.104:80"

Alarms would be raised on enpoints watchers and childchain enpoints: /alarm.get. Perhaps logged as well.

I think the issue is in Nginx. during reorgs when one node is paused, Nginx still tries to access it

priv/cabbage/apps/itest/lib/client.ex

InoMurko · 2020-07-06T09:02:27Z

priv/cabbage/apps/itest/lib/poller.ex

@@ -295,6 +291,10 @@ defmodule Itest.Poller do
        Process.sleep(@sleep_retry_sec)
        submit_typed(typed_data_signed, counter - 1)

+      %{"messages" => %{"code" => "operation:service_unavailable"}} ->


it didn't dissapear?

InoMurko · 2020-07-06T09:10:00Z

priv/cabbage/apps/itest/lib/account.ex

  end

-  defp hash(message), do: ExthCrypto.Hash.hash(message, ExthCrypto.Hash.kec())
+  def with_retries(func, total_time \\ 510, current_time \\ 0) do


mixing private and public functions.
also, what is the reason we need to retry. Is it so that all nodes unlock an account? is there a better way to do this?

I didn't find any other way. It is related to Nginx timeout. Some requests can no finish in 10 seconds. I set it 10 because during reorg Nginx continues to redirect to the paused node (til the first failure?), paused node accepts a request but do not return any response for N (timeout limit) seconds

.circleci/config.yml

InoMurko · 2020-07-07T10:16:19Z

priv/cabbage/apps/itest/lib/plasma_framework.ex

@@ -96,4 +100,19 @@ defmodule Itest.PlasmaFramework do
      Map.put(acc, key, value)
    end)
  end
+
+  def with_retries(func, total_time \\ 120, current_time \\ 0) do


mixing private and public functions

InoMurko · 2020-07-07T10:34:53Z

priv/cabbage/apps/itest/test/itest/deposits_test.exs

+        amount
+        |> Currency.to_wei()
+        |> Client.deposit(alice_account, Itest.PlasmaFramework.vault(Currency.ether()))
+      end)


after the anon. function, the balance = Client.get_exact_balance(alice_account, expecting_amount) would def. return the correct and LAST balance because the reorg is at this point finished, correct?

not really, it takes some time for childchain to sync the latest data

InoMurko · 2020-07-07T10:35:56Z

priv/cabbage/apps/itest/lib/reorg.ex

+    if Application.get_env(:cabbage, :reorg) do
+      pause_container!(@node1)
+      unpause_container!(@node2)
+
+      Process.sleep(10_000)
+
+      func.()
+
+      Process.sleep(10_000)
+
+      pause_container!(@node2)
+      unpause_container!(@node1)
+
+      Process.sleep(30_000)
+
+      response = func.()
+
+      Process.sleep(30_000)
+
+      unpause_container!(@node2)
+      unpause_container!(@node1)
+
+      :ok = Poller.wait_until_peer_count(1)
+
+      Process.sleep(20_000)
+
+      response


I don't like these sleeps, because they're undeterministic. Is there a way for us to make this less prone to error?

I can request the block number from nodes and wait for 5 generated blocks. I'll do this now.

InoMurko · 2020-07-07T10:58:09Z

I've noticed another issue:

elixir-omg/priv/cabbage/apps/itest/test/itest/deposits_test.exs

Lines 61 to 88 in f713eb6

    
           defthen ~r/^Alice should have "(?<amount>[^"]+)" ETH on the child chain$/, 
        
                   %{amount: amount}, 
        
                   %{alice_account: alice_account} = state do 
        
             geth_block_every = 1 
        
             {:ok, response} = 
        
               WatcherSecurityCriticalAPI.Api.Configuration.configuration_get(WatcherSecurityCriticalAPI.Connection.new()) 
        
             watcher_security_critical_config = 
        
               WatcherSecurityCriticalConfiguration.to_struct(Jason.decode!(response.body)["data"]) 
        
             finality_margin_blocks = watcher_security_critical_config.deposit_finality_margin 
        
             to_miliseconds = 1000 
        
             finality_margin_blocks 
        
             |> Kernel.*(geth_block_every) 
        
             |> Kernel.*(to_miliseconds) 
        
             |> Kernel.round() 
        
             |> Process.sleep() 
        
             expecting_amount = Currency.to_wei(amount) 
        
             balance = Client.get_exact_balance(alice_account, expecting_amount) 
        
             balance = balance["amount"] 
        
             assert_equal(expecting_amount, balance, "For #{alice_account}") 
        
             {:ok, state} 
        
           end

geth_block_every = 1 is basically - we assume a block definetly arrives every second (which worked fine with clique).

Perhaps a better approach, scalable at least, would be to get the curent ethereum height and compare it with curent ethereum height + watcher_security_critical_config.deposit_finality_margin, when the current ethereum height is above curent ethereum height + watcher_security_critical_config.deposit_finality_margin you're allowed to check for balance.

Or a even better approach, would be to get the block number at which the deposit transaction was made and compare it with curent ethereum height + watcher_security_critical_config.deposit_finality_margin, when the deposit ethereum height is above curent ethereum height + watcher_security_critical_config.deposit_finality_margin you're allowed to check for balance.

ayrat555 · 2020-07-08T08:18:40Z

@InoMurko how does it look?

ayrat555 force-pushed the add-reorged-docker-compose branch 4 times, most recently from 9330f31 to 4e3b5c8 Compare June 14, 2020 20:53

InoMurko suggested changes Jun 15, 2020

View reviewed changes

docker-compose-reorg.yml Outdated Show resolved Hide resolved

docker-compose-reorg.yml Outdated Show resolved Hide resolved

docker-compose-reorg.yml Outdated Show resolved Hide resolved

docker-compose-reorg.yml Outdated Show resolved Hide resolved

ayrat555 force-pushed the add-reorged-docker-compose branch from c22055b to b4891f5 Compare June 16, 2020 15:22

InoMurko suggested changes Jun 17, 2020

View reviewed changes

priv/cabbage/apps/itest/lib/reorg.ex Show resolved Hide resolved

ayrat555 force-pushed the add-reorged-docker-compose branch 4 times, most recently from fd0abb8 to e5a54b8 Compare June 18, 2020 12:52

ayrat555 requested review from InoMurko, unnawut, achiurizo and mederic-p June 18, 2020 13:40

unnawut approved these changes Jun 18, 2020

View reviewed changes

priv/cabbage/apps/itest/lib/account.ex Outdated Show resolved Hide resolved

InoMurko suggested changes Jun 22, 2020

View reviewed changes

add reorged docker-compose config

201593c

ayrat555 force-pushed the add-reorged-docker-compose branch from 8abc82c to 201593c Compare June 24, 2020 07:20

add hack for reorg tests

1ef4b25

ayrat555 force-pushed the add-reorged-docker-compose branch from 2870dac to 75acd8d Compare June 24, 2020 10:31

migrate some tests to new reorg interface

73a6cdb

ayrat555 force-pushed the add-reorged-docker-compose branch from 75acd8d to 73a6cdb Compare June 24, 2020 10:53

send empty transactions

22734ca

ayrat555 force-pushed the add-reorged-docker-compose branch from 4cbfa8f to 22734ca Compare June 24, 2020 14:25

ayrat555 added 2 commits June 24, 2020 17:55

return wait period after reorg

47e473f

add sleep after pausing the first node so reorg has time

3ba3fa6

ayrat555 force-pushed the add-reorged-docker-compose branch from 1e389e5 to 3ba3fa6 Compare June 24, 2020 15:22

finish reorgs in on_exit callback

c392467

ayrat555 added 3 commits July 5, 2020 13:32

make release cabbage tests pass

104f863

make reorg and consensus block numbers different

2ec0d1a

make reorg tests run faster

8da463d

ayrat555 requested a review from InoMurko July 6, 2020 06:58

InoMurko suggested changes Jul 6, 2020

View reviewed changes

InoMurko reviewed Jul 6, 2020

View reviewed changes

ayrat555 force-pushed the add-reorged-docker-compose branch from 9ed6bb0 to 49976e5 Compare July 6, 2020 12:00

code review comments

240005e

ayrat555 force-pushed the add-reorged-docker-compose branch from 49976e5 to 240005e Compare July 6, 2020 12:33

ayrat555 added 4 commits July 6, 2020 15:48

reorg failure

797e931

add poller changes

2b3f76c

add sleep after pausing/unpausing

d26b350

cr issues

f713eb6

ayrat555 requested a review from InoMurko July 6, 2020 17:03

InoMurko suggested changes Jul 7, 2020

View reviewed changes

ayrat555 added 3 commits July 7, 2020 17:56

fetch block numbers instead of Process.sleep

69abb70

fetch transaction block_number

3d2307b

Merge branch 'master' into add-reorged-docker-compose

7028468

ayrat555 requested a review from InoMurko July 7, 2020 16:19

ayrat555 added 2 commits July 7, 2020 19:43

return get_exact_balance

0eb77f2

check Alice's balance on the root chain after deposits

ccd900b

ayrat555 added 2 commits July 8, 2020 13:33

remove unused retries

593b8f6

Merge branch 'master' into add-reorged-docker-compose

f8d1f81

InoMurko approved these changes Jul 8, 2020

View reviewed changes

ayrat555 merged commit 8aebb53 into master Jul 9, 2020

ayrat555 deleted the add-reorged-docker-compose branch July 9, 2020 07:41

unnawut added the chore Technical work that does not affect service behaviour label Aug 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add reorged docker compose #1579

Add reorged docker compose #1579

ayrat555 commented Jun 13, 2020 •

edited

Loading

ayrat555 commented Jun 18, 2020

unnawut left a comment

InoMurko Jun 22, 2020

ayrat555 Jun 24, 2020

ayrat555 Jun 25, 2020 •

edited

Loading

ayrat555 commented Jul 4, 2020 •

edited

Loading

InoMurko Jul 6, 2020

InoMurko Jul 6, 2020

InoMurko Jul 6, 2020

ayrat555 Jul 6, 2020

InoMurko Jul 6, 2020 •

edited

Loading

ayrat555 Jul 6, 2020

InoMurko Jul 6, 2020

ayrat555 Jul 6, 2020

InoMurko Jul 6, 2020

InoMurko Jul 6, 2020

ayrat555 Jul 6, 2020

InoMurko Jul 7, 2020

InoMurko Jul 7, 2020

ayrat555 Jul 7, 2020

InoMurko Jul 7, 2020

ayrat555 Jul 7, 2020 •

edited

Loading

InoMurko commented Jul 7, 2020

ayrat555 commented Jul 8, 2020

Add reorged docker compose #1579

Add reorged docker compose #1579

Conversation

ayrat555 commented Jun 13, 2020 • edited Loading

Overview

ayrat555 commented Jun 18, 2020

unnawut left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayrat555 Jun 25, 2020 • edited Loading

Choose a reason for hiding this comment

ayrat555 commented Jul 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

InoMurko Jul 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayrat555 Jul 7, 2020 • edited Loading

Choose a reason for hiding this comment

InoMurko commented Jul 7, 2020

ayrat555 commented Jul 8, 2020

ayrat555 commented Jun 13, 2020 •

edited

Loading

ayrat555 Jun 25, 2020 •

edited

Loading

ayrat555 commented Jul 4, 2020 •

edited

Loading

InoMurko Jul 6, 2020 •

edited

Loading

ayrat555 Jul 7, 2020 •

edited

Loading