Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rng reporter hanging #447

Merged
merged 13 commits into from
Nov 23, 2022
Merged

Fix rng reporter hanging #447

merged 13 commits into from
Nov 23, 2022

Conversation

oraclown
Copy link
Collaborator

Summary

Steps Taken to QA Changes

  • Added test for data source failure: tests/reporters/test_rng_reporter.py::test_missing_blockhash. This checks to make sure it attempts to report again after a source failure (like the one shown in the linked issue's error logs).

Checklist

This pull request is:

  • A documentation error, docs update, or typographical error fix
    • No tests or issue needed
  • A code fix
    • Please reference the related issue by including "Closes <link to issue>" in this Pull Request's summary section.
      • If no issue exists, please create a bug report issue
    • Please include tests. Fixes without tests will not be accepted unless it's related to the documentation only.
    • Please make sure docs are updated if need be
  • A feature implementation
    • Please reference the related issue by including "Closes <link to issue>" in this Pull Request's summary section.
      • If no issue exists, please create a feature enhancement issue
    • Please include tests
    • Please make sure docs updates are both thorough and easy to reproduce by somebody with limited knowledge of the feature that you are submitting

Happy engineering!

@oraclown oraclown requested a review from akremstudy November 21, 2022 14:38
@codecov-commenter
Copy link

codecov-commenter commented Nov 21, 2022

Codecov Report

Merging #447 (1bb3cc6) into main (510ada4) will increase coverage by 60.65%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##             main     #447       +/-   ##
===========================================
+ Coverage   27.93%   88.59%   +60.65%     
===========================================
  Files          97       97               
  Lines        3254     3270       +16     
===========================================
+ Hits          909     2897     +1988     
+ Misses       2345      373     -1972     
Impacted Files Coverage Δ
tests/reporters/test_custom_360_reporter.py 100.00% <ø> (+82.60%) ⬆️
tests/reporters/test_interval_reporter.py 96.13% <ø> (+70.71%) ⬆️
tests/reporters/test_polygon_reporter.py 91.52% <ø> (+62.71%) ⬆️
tests/reporters/test_uspce_reporter.py 89.28% <ø> (+57.14%) ⬆️
tests/conftest.py 93.16% <100.00%> (+46.35%) ⬆️
tests/reporters/test_360reporter.py 100.00% <100.00%> (+93.97%) ⬆️
tests/reporters/test_rng_reporter.py 96.07% <100.00%> (+78.43%) ⬆️
tests/utils/test_reporter_utils.py 83.07% <100.00%> (+69.23%) ⬆️
.../integrations/diva_protocol/test_diva_contracts.py 35.00% <0.00%> (+13.75%) ⬆️
tests/tips/test_multicall_autopay.py 48.93% <0.00%> (+14.89%) ⬆️
... and 89 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Collaborator

@akremstudy akremstudy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @oraclown, nice fixes and changes. These tests are failing for me and not sure why, are they passing for you? I guess the most relevant one to this pr is the first. Everything looks good to me otherwise.
tests/reporters/test_360reporter.py::test_no_native_token
tests/reporters/test_custom_360_reporter.py::test_submit_once tests/reporters/test_polygon_reporter.py

tests/reporters/test_rng_reporter.py Show resolved Hide resolved
@@ -170,5 +138,4 @@ async def test_no_native_token(tellor_360, caplog):

await reporter.report(report_count=1)

expected = f"Account {account.address} has insufficient native token funds".lower()
assert expected in caplog.text.lower()
assert "insufficient native token funds" in caplog.text.lower()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test is failing here

@oraclown
Copy link
Collaborator Author

@akremstudy tests/reporters/test_360reporter.py::test_no_native_token passes for me:

(env) ➜  telliot-feeds git:(fix-bug-432) pytest tests/reporters/test_360reporter.py::test_no_native_token
====================================================================== test session starts ======================================================================
platform darwin -- Python 3.9.7, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
rootdir: /Users/owen/tellor/telliot-feeds, configfile: pyproject.toml
plugins: eth-brownie-1.19.0, forked-1.4.0, asyncio-0.19.0, web3-5.27.0, dotenv-0.5.2, xdist-1.34.0, hypothesis-6.27.3, cov-3.0.0
asyncio: mode=strict
collected 1 item                                                                                                                                                

Launching 'ganache-cli --port 8545 --gasLimit 12000000 --accounts 10 --hardfork istanbul --mnemonic brownie'...

tests/reporters/test_360reporter.py::test_no_native_token 
------------------------------------------------------------------------ live log setup -------------------------------------------------------------------------
INFO     telliot_core:versions.py:10 telliot-core 0.1.7dev0
INFO     telliot_core:core.py:245 Connected to polygon-mumbai [default account: _test_account], time: 2022-11-22 07:06:10.962066
ERROR    telliot_core.contract.contract:response.py:34 Send transaction failed: ValueError({'message': 'VM Exception while processing transaction: revert only owner can set governance address', 'code': -32000, 'data': {'0x86b4e87f698fb1e854652b1b8e569beeaf4e82cfaea232adf7a0c0eaaf924f8b': {'error': 'revert', 'program_counter': 1681, 'return': '0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000256f6e6c79206f776e65722063616e2073657420676f7665726e616e63652061646472657373000000000000000000000000000000000000000000000000000000', 'reason': 'only owner can set governance address'}, 'stack': 'c: VM Exception while processing transaction: revert only owner can set governance address\n    at Function.c.fromResults (/Users/owen/.nvm/versions/node/v17.4.0/lib/node_modules/ganache-cli/build/ganache-core.node.cli.js:4:192416)\n    at w.processBlock (/Users/owen/.nvm/versions/node/v17.4.0/lib/node_modules/ganache-cli/build/ganache-core.node.cli.js:42:50915)\n    at processTicksAndRejections (node:internal/process/task_queues:96:5)', 'name': 'c'}})
------------------------------------------------------------------------- live log call -------------------------------------------------------------------------
INFO     telliot_feeds.reporters.tellorflex:tellorflex.py:79 Reporting with account: 0x3D79****
WARNING  telliot_feeds.utils.reporter_utils:reporter_utils.py:88 Insufficient native token funds for 0x3D79****. Balance: 1.00. Expected: 2.00
INFO     telliot_feeds.reporters.interval:interval.py:459 Sleeping for 0 seconds
PASSED                                                                                                                                                    [100%]

======================================================================= 1 passed in 4.11s =======================================================================
Terminating local RPC client...

@oraclown
Copy link
Collaborator Author

Added init.py file to reporters/rewards/ bc of test error:

     from telliot_feeds.reporters.tellor_360 import Tellor360Reporter
  .tox/py39/lib/python3.9/site-packages/telliot_feeds/reporters/tellor_360.py:15: in <module>
      from telliot_feeds.reporters.rewards.time_based_rewards import get_time_based_rewards
  E   ModuleNotFoundError: No module named 'telliot_feeds.reporters.rewards'

Made a separate issue for the other less-related tests #451

@oraclown
Copy link
Collaborator Author

tests/reporters/test_polygon_reporter.py

this one is unrelated:
tests/reporters/test_polygon_reporter.py::test_fetch_gas_price_error

@oraclown
Copy link
Collaborator Author

Thanks @oraclown, nice fixes and changes. These tests are failing for me and not sure why, are they passing for you? I guess the most relevant one to this pr is the first. Everything looks good to me otherwise. tests/reporters/test_360reporter.py::test_no_native_token tests/reporters/test_custom_360_reporter.py::test_submit_once tests/reporters/test_polygon_reporter.py

This one tests/reporters/test_custom_360_reporter.py::test_submit_once is failing bc of #445 I think:

INFO     telliot_feeds.reporters.tellor_360:response.py:34 Unable to read current stake amount
WARNING  telliot_feeds.reporters.interval:interval.py:317 Unable to read current stake amount
==================================================================== short test summary info ====================================================================
FAILED tests/reporters/test_custom_360_reporter.py::test_submit_once - AssertionError: assert False

@oraclown oraclown merged commit 0799e84 into main Nov 23, 2022
@oraclown oraclown deleted the fix-bug-432 branch November 23, 2022 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Telliot stuck with "Unable to retrieve updated datafeed value."
3 participants