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

[WIP] Support events watching from specific contracts #14

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AwaitFuture
Copy link
Contributor

Problem:

  • Lacked functionality listen to specific addresses for approvals

Solution:

  • Add some logic to make handle it :^)

should solve most of #4

Open questions:

  • I added a different config file I think is useful as a demonstration / interacting with test networks, let me know if I should remove it.
  • These changes lack any testing; I intend to add some tests to at least confirm that the config parser produces the correct results, is there something else I should test aswell?
  • address in log_subscription is forced into having empty string as the default case, which avoids doing any address specific logic. is there a better way to handle this? it seems necessary to at least have a default address value, since log_subscriptions ETS storage depends on it to distinguish between different log_subscriptions.

lib/slurp/logs/log_fetcher.ex Outdated Show resolved Hide resolved
lib/slurp/logs/subscriptions.ex Show resolved Hide resolved
Copy link
Contributor

@rupurt rupurt left a comment

Choose a reason for hiding this comment

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

This is looking really good @AwaitFuture 🦸

Testing is definitely lacking across the project and now that there are 2 of us I'll start adding them. A config parser test seems fine for now as there isn't any kind of reasonable test structure for you to add onto atm.

config/rinkeby.exs Outdated Show resolved Hide resolved
lib/slurp/logs/log_fetcher.ex Outdated Show resolved Hide resolved
lib/slurp/logs/subscriptions.ex Show resolved Hide resolved
lib/slurp/logs/log_fetcher.ex Outdated Show resolved Hide resolved
lib/slurp/logs/log_fetcher.ex Outdated Show resolved Hide resolved
lib/slurp/logs/subscriptions.ex Outdated Show resolved Hide resolved
@rupurt rupurt force-pushed the main branch 2 times, most recently from 3dd6ce6 to 15f4a22 Compare June 22, 2021 19:16
@AwaitFuture AwaitFuture force-pushed the specific_address branch 6 times, most recently from a68107d to be54b53 Compare August 25, 2021 17:53
@AwaitFuture
Copy link
Contributor Author

It seems like theres some transient failures on some tests; running individual tests seems to fail locally, but running all tests together causes them to pass most of the time. I looked through for a bit to see if I could sus out what is persisting from some test that allows all the others to pass, but Im not really sure.

@rupurt
Copy link
Contributor

rupurt commented Aug 27, 2021

Thanks for the updates @AwaitFuture. I'll have to take a look at it locally

@AwaitFuture
Copy link
Contributor Author

@rupurt I set the test blockchain in the runtime config to launch on start; somehow the poll interval for new heads gets set to nil if you do this sometimes. reverting that fixed it.

@AwaitFuture AwaitFuture requested a review from rupurt August 29, 2021 04:25
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.

2 participants