nanocurrency / nano-node

Nano is digital currency. Its ticker is: XNO and its currency symbol is: Ӿ
https://nano.org
BSD 3-Clause "New" or "Revised" License
3.49k stars 787 forks source link

Unit test rpc.epoch_upgrade failed on macos CI #3476

Closed dsiganos closed 3 years ago

dsiganos commented 3 years ago
[ RUN      ] rpc.epoch_upgrade
/Users/runner/work/nano-node/nano-node/nano/rpc_test/rpc.cpp:5668: Failure
Expected equality of these values:
  "0"
  response_fail.get<std::string> ("started")
    Which is: "1"
[  FAILED  ] rpc.epoch_upgrade (1070 ms)
dsiganos commented 3 years ago

The second RPC call to epoch_upgrade looks like a race condition to me. If I understand well what is happening, the second RPC call is checking that the first call is in progress. But that is a transient state and it is not guaranteed that the test will notice that transient state. So it is a race condition.

If I add a one second sleep between the forst RPC and the second RPC call then the unit test always fails.

    request.put ("action", "epoch_upgrade");
    request.put ("epoch", 1);
    request.put ("key", epoch_signer.prv.to_string ());
    auto response (wait_response (system, rpc, request));
    ASSERT_EQ ("1", response.get<std::string> ("started"));

    // **** ADDING THIS MAKES IT FAIL EVERY TIME *****
    sleep(1); 

    auto response_fail (wait_response (system, rpc, request));
    ASSERT_EQ ("0", response_fail.get<std::string> ("started")); 
dsiganos commented 3 years ago

Documentation epoch_upgrade RPC: "The RPC command epoch_upgrade spawns a background task to iterate over all accounts and add the epoch block to any accounts that do not have it. It will return { "started" = "1" } if the background task was spawned successfully. It will return { "started" = "0" } if the operation could not be started. Reasons for not being able to start the operations include the node being stopped and a previous being in progress. "epoch" can be set to either 1 or 2."

zhyatt commented 3 years ago

Docs updated in https://github.com/nanocurrency/nano-docs/pull/576