Skip to content
This repository has been archived by the owner on Aug 14, 2019. It is now read-only.

Implement %eth-watcher, an app for tracking Ethereum events #1081

Merged
merged 6 commits into from
Feb 26, 2019
Merged

Conversation

Fang-
Copy link
Member

@Fang- Fang- commented Feb 22, 2019

This is more or less a port of jael's Ethereum watching logic, without the Azimuth-specific transformations and with some extra niceties to make this generically usable.

Poke with [%watch %sometag config:eth-watcher] to initialize,
then subscribe at /sometag to receive updates in the shape of
[%snap snapshot:eth-watcher] for initial and on-reorg logs,
[%logs loglist] for logs as they happen.

I haven't thoroughly tested the peer/subscription functionality yet, but an app that uses this to build Azimuth statistics is next on my to-do list. That'll both hit that, and tell me whether there's any functionality that is still missing from this.
So, ready for review, but might see a small addition or two.

Poke with [%watch %sometag config:eth-watcher] to initialize,
then subscribe at /sometag to receive updates in the shape of
[%snap snapshot:eth-watcher] for initial and on-reorg logs,
[%logs loglist] for logs as they happen.
@Fang- Fang- added the apps label Feb 22, 2019
app/eth-watcher.hoon Outdated Show resolved Hide resolved
@ohAitch
Copy link
Contributor

ohAitch commented Feb 22, 2019 via email

Copy link
Contributor

@philipcmonk philipcmonk left a comment

Choose a reason for hiding this comment

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

A couple of small things that should be addressed, but mostly stylistic comments, take or leave those.

Appears to be about as good as the %jael implementation. If you find ways to make this more robust, please port to %jael as well!

app/eth-watcher.hoon Outdated Show resolved Hide resolved
app/eth-watcher.hoon Show resolved Hide resolved
[t.wir res]
::
++ watcher
|_ $: =name
Copy link
Contributor

Choose a reason for hiding this comment

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

The |_/++open/++init pattern is interesting, I've usually seen it as an explicit =|/|% to communicate that you're not meant to replace the sample.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've seen that before in places. I prefer |_ in that it very clearly communicates that "these things are used/changed inside this core". I agree this does leave the sample there easily touchable, which may add some confusion back in...

app/eth-watcher.hoon Outdated Show resolved Hide resolved
app/eth-watcher.hoon Show resolved Hide resolved
app/eth-watcher.hoon Outdated Show resolved Hide resolved
logs.eye logs.snap
==
--
--
Copy link
Contributor

Choose a reason for hiding this comment

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

need a trailing newline

Copy link
Member Author

Choose a reason for hiding this comment

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

"Need"? Is there something that would be tripped up by not having trailing newlines?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, isn't that what sunk ~zod one of those times? We might manage to parse it for gall apps, but at least hoon.hoon really needs a trailing newline or it won't compile. Plus, a text file with no trailing newline is really malformed.

sur/eth-watcher.hoon Outdated Show resolved Hide resolved
lib/eth-watcher.hoon Outdated Show resolved Hide resolved
@@ -0,0 +1,38 @@
:: eth-watcher utilities
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these two in a separate lib? Do we expect clients to use them?

Copy link
Member Author

Choose a reason for hiding this comment

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

I imagine clients may want ++store-new-logs, yes. ++log-to-id is just there to support it, though maybe it belongs in zuse instead?

@Fang- Fang- merged commit 8afb617 into next Feb 26, 2019
@Fang- Fang- deleted the eth-watcher branch February 26, 2019 10:11
@ixv ixv mentioned this pull request Mar 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants