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

Consolidate #684

Merged
merged 40 commits into from
Aug 2, 2023
Merged

Consolidate #684

merged 40 commits into from
Aug 2, 2023

Conversation

akremstudy
Copy link
Collaborator

@akremstudy akremstudy commented Jun 11, 2023

Summary

Made quite a few edits:

  • removed deprecated TellorX mock solidtity contracts and SampleXReporter
  • Edited the SampleFlexReporter solidity contract to work and fixed test, this wasn't working before
  • Added base_fee flag to allow user to input that in the cli if they wish (its another way to estimate the max fee)
  • Added a request-withdraw and a stake command to be able to call the respective methods in the oracle contract separate from the reporter command.
  • Separate common command options into two See here
  • Decreased the max priority fee (aka tip) default range to 3 gwei since 80 is too high.
  • Adjust Diva reporter to use GasFees object to generate gas fields when reporting and also consolidate the code to only adapt the method it needs instead of overwriting the report_once and report methods.
  • Add stake command
  • Same as diva with Flashbots reporter
  • Remove redundant code in RNG reporter.
  • Added GasFees class that would be inherited and used to generated gas params for the reporter.
  • Add StakeInfo class to be used in the reporter to monitor when stake amount is changed or reporter is slashed currently the reporter will auto stake if any of these events happen.
  • Broke up some of the code in the reporter for easier readability and usage

Steps Taken to QA Changes

  • Added new tests for the new classes plus another reporter test to check the changes
  • Consolidated the previous reporter tests into one
  • Fixed tests that were broken due to the changes

Example transactions using cli:

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!

@akremstudy
Copy link
Collaborator Author

@oraclown, Can I get a review on this pr specifically these two files reporter and GasFees. Made a lot of changes but those are the main ones, if you can review them first then I can make changes before you spend time on the other files.

@akremstudy akremstudy requested a review from oraclown July 3, 2023 14:18
Copy link
Collaborator

@oraclown oraclown left a comment

Choose a reason for hiding this comment

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

Thanks, @akremstudy ! Awesome refactoring of the reporter. Huge. I has some minor suggestions. Some tests needed for the new cli commands. I'd also encourage posting some example txs using the new cli commands, as well as just reporting, just as a sanity check. Great cli additions and updates to gas fees calculation code!

src/telliot_feeds/cli/utils.py Show resolved Hide resolved
src/telliot_feeds/cli/commands/request_withdraw_stake.py Outdated Show resolved Hide resolved
src/telliot_feeds/cli/commands/request_withdraw_stake.py Outdated Show resolved Hide resolved
tests/reporters/test_gas_fees.py Show resolved Hide resolved
src/telliot_feeds/reporters/tellor_360.py Show resolved Hide resolved
src/telliot_feeds/reporters/tellor_360.py Show resolved Hide resolved
tests/reporters/test_360_reporter.py Outdated Show resolved Hide resolved
tests/reporters/test_360_reporter.py Show resolved Hide resolved
@0xSpuddy
Copy link
Contributor

Testing by running a reporter on each testnet listening for tips / reporting at regular intervals. I haven't tested every combination of flags, but no errors so far.

@akremstudy akremstudy marked this pull request as ready for review July 25, 2023 14:08
Copy link
Collaborator

@oraclown oraclown left a comment

Choose a reason for hiding this comment

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

Thanks for the updates, @akremstudy !

@akremstudy akremstudy merged commit 654e65b into main Aug 2, 2023
4 checks passed
@akremstudy akremstudy deleted the consolidate branch August 2, 2023 13:07
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.

3 participants