Update creating an event transformer docs by paytonrules · Pull Request #182 · sky-ecosystem/vdb-mcd-transformers

paytonrules · 2020-04-29T22:46:12Z

No description provided.

rmulhol

Nice! 🙏 🙏 🙏

I left a bunch of comments but they may be extraneous - curious what you think

rmulhol · 2020-04-29T23:46:54Z

transformers/events/DOCUMENTATION.md

+If the contract isn't already present in the environment you'll need to add it
+before you can begin transforming its events. To do so:
+
+1. Get the contract signature if you don't already have it.


the term we'd want to use here is contract address

rmulhol · 2020-04-29T23:47:17Z

transformers/events/DOCUMENTATION.md

+1. Get the contract signature if you don't already have it.
+1. Search for the contract on [etherscan](https://etherscan.io/). Bookmark this
+   page, it is your new best friend.
+1. In each environment (at this time testing, docker and mcdTransformers) add a


no oxford comma?!

Don't tell James.

rmulhol · 2020-04-29T23:49:01Z

transformers/events/DOCUMENTATION.md

+      abi      = '[{"inputs":[{"internalType":"address","name":"vat_","type":"address"},{"internalType":"bytes32","name":"ilk_","type":"bytes32"}],"payable":false,"stateMutability":"nonpayable","type":"constructor"},{"anonymous":false,"inputs":[{"indexed":false,"internalType":"uint256","name":"id","type":"uint256"},{"indexed":false,"internalType":"uint256","name":"lot","type":"uint256"},{"indexed":false,"internalType":"uint256","name":"bid","type":"uint256"},{"indexed":false,"internalType":"uint256","name":"tab","type":"uint256"},{"indexed":true,"internalType":"address","name":"usr","type":"address"},{"indexed":true,"internalType":"address","name":"gal","type":"address"}],"name":"Kick","type":"event"},{"anonymous":true,"inputs":[{"indexed":true,"internalType":"bytes4","name":"sig","type":"bytes4"},{"indexed":true,"internalType":"address","name":"usr","type":"address"},{"indexed":true,"internalType":"bytes32","name":"arg1","type":"bytes32"},{"indexed":true,"internalType":"bytes32","name":"arg2","type":"bytes32"},{"indexed":false,"internalType":"bytes","name":"data","type":"bytes"}],"name":"LogNote","type":"event"},{"constant":true,"inputs":[],"name":"beg","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[{"internalType":"uint256","name":"","type":"uint256"}],"name":"bids","outputs":[{"internalType":"uint256","name":"bid","type":"uint256"},{"internalType":"uint256","name":"lot","type":"uint256"},{"internalType":"address","name":"guy","type":"address"},{"internalType":"uint48","name":"tic","type":"uint48"},{"internalType":"uint48","name":"end","type":"uint48"},{"internalType":"address","name":"usr","type":"address"},{"internalType":"address","name":"gal","type":"address"},{"internalType":"uint256","name":"tab","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"internalType":"uint256","name":"id","type":"uint256"}],"name":"deal","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"internalType":"uint256","name":"id","type":"uint256"},{"internalType":"uint256","name":"lot","type":"uint256"},{"internalType":"uint256","name":"bid","type":"uint256"}],"name":"dent","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"internalType":"address","name":"usr","type":"address"}],"name":"deny","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"internalType":"bytes32","name":"what","type":"bytes32"},{"internalType":"uint256","name":"data","type":"uint256"}],"name":"file","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[],"name":"ilk","outputs":[{"internalType":"bytes32","name":"","type":"bytes32"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"internalType":"address","name":"usr","type":"address"},{"internalType":"address","name":"gal","type":"address"},{"internalType":"uint256","name":"tab","type":"uint256"},{"internalType":"uint256","name":"lot","type":"uint256"},{"internalType":"uint256","name":"bid","type":"uint256"}],"name":"kick","outputs":[{"internalType":"uint256","name":"id","type":"uint256"}],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[],"name":"kicks","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"internalType":"address","name":"usr","type":"address"}],"name":"rely","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[],"name":"tau","outputs":[{"internalType":"uint48","name":"","type":"uint48"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"internalType":"uint256","name":"id","type":"uint256"},{"internalType":"uint256","name":"lot","type":"uint256"},{"internalType":"uint256","name":"bid","type":"uint256"}],"name":"tend","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"internalType":"uint256","name":"id","type":"uint256"}],"name":"tick","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[],"name":"ttl","outputs":[{"internalType":"uint48","name":"","type":"uint48"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[],"name":"vat","outputs":[{"internalType":"contract VatLike","name":"","type":"address"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[{"internalType":"address","name":"","type":"address"}],"name":"wards","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"internalType":"uint256","name":"id","type":"uint256"}],"name":"yank","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"}]'
+      deployed = 8928180
+```
+* The address is the signature you searched for, as a string.


I recognize it sounds weird to say "The address is the address" but I think the most idiomatic description might be something like "The address field should contain the hex representation of the address you used to find the contract on Etherscan," or something like that

How about "The address field is the contract address you searched for, as a string."

rmulhol · 2020-04-29T23:52:43Z

transformers/events/DOCUMENTATION.md

+like above.
+* The deployed field is the block number the contract was deployed on. That
+can be found on the Contract's page, by looking for the Contract Creator
+and clicking it's transaction hash. That will take you to the block it was deployed on.


Technically it takes you to the transaction where it was deployed, which includes a link to the block that contains that transaction 🤓

rmulhol · 2020-04-29T23:55:00Z

transformers/events/DOCUMENTATION.md

+(the raw, untransformed data) to an `event.InsertionModel` (the domain object).
+
+Note that for this step you probably do _not_ need to create the database
+migration. This is because the only thing you'll be saving are addresses, which


It's possible you could be saving other things that are stored in other tables and referenced via foreign key, like maker.ilks or maker.urns - but I think the larger point stands that you shouldn't need a migration to make the transformer, as the only thing a transformer inserts are fields referenced by FK

I added some clarifying language here.

rmulhol · 2020-04-29T23:59:29Z

transformers/events/DOCUMENTATION.md

+	RunSpecs(t, "<PackageName>  Suite")
+}
+```
+   1. Implement the toModels function in `transformer.go` with the appropriate


:%s/toModels/ToModels

Maybe helpful to include a link to the interface?

rmulhol · 2020-04-30T00:00:34Z

transformers/events/DOCUMENTATION.md

+   into a go struct. 
+   1. For custom events you can convert each EventLog entry into an entity
+   using the function `contract.UnpackLog(&entity, "<EventLogName>", log.Log)`.
+   The entity is usually defined (by you) in a file called `entity.go` in the


"by you" is probably the right answer here although in theory abigen may be helpful/necessary if figuring out the type mappings is challenging

I added a note about abigen

rmulhol · 2020-04-30T00:02:08Z

transformers/events/DOCUMENTATION.md

+1. Use the `make new_migration` task to create a new migration.
+    * Each event log has its own table in the database.
+    * The specific log event tables are all created in the `maker` schema.
+1. The new migration can be run by running `make test` or `make migration NAME=vulcanize_test`.


maybe worth replacing vulcanize_test with <database_name>?

rmulhol · 2020-04-30T00:03:17Z

transformers/events/DOCUMENTATION.md

+1. The new migration can be run by running `make test` or `make migration NAME=vulcanize_test`.
+1. To verify that the migration will work create an integration for the shiny
+   new transformer in [`integration_tests`](../integration_tests).
+1. Integration tests can be run with `make integrationtest`.


I sorta shudder at the thought of describing it in detail because it's something we need to streamline, but maybe worth covering something about what goes into writing an integration test for an event transformer?

Probably - I think I was running into some documentation fatigue at this point.

rmulhol · 2020-04-30T00:04:33Z

transformers/events/DOCUMENTATION.md

+
+Simply replace the constants and package names with your transformer.
+
+1. Finally add your package to the list of transformerExporters in `plugins/transformerExporter.go`. Again alphabetically.


maybe extraneous but you can generate the transformerExporter.go from the command line with ./vulcanizedb compose --config=/path/to/config.toml

Didn't know that!

elizabethengelman

This is looking great! It's so nice to have accurate documentation again!

Just a few small comments, but nothing merge blocking.

elizabethengelman · 2020-04-30T20:10:42Z

transformers/events/DOCUMENTATION.md

+migration, you'll know, because the tests won't pass. The directions here assume
+you do not, yet.
+
+1. Search for the contract on etherscan using it's signature. 


I think this signature should be address here as well.

elizabethengelman · 2020-04-30T20:12:56Z

transformers/events/DOCUMENTATION.md

+          thank me later.
+        * Update constants as needed. Look at older examples of test data for
+          inspiration.
+        * You do not need a database migration at this point.


I wonder if this bullet point about a migration is necessary yet, since it's already mentioned above that a migration isn't likely necessary on this step.

elizabethengelman · 2020-04-30T20:18:36Z

transformers/events/DOCUMENTATION.md

+   1. After converting the log entry to an entity, look up all of it's foreign
+      keys from it's data and assign them to the entity.
+   1. From the entity create an InsertionModel, which you return.
+   1. For `LogNote` events you'll need to look at the method signature of the


I wonder if it would be helpful to have directions for LogNote and directions for custom events as nested bullets to signify that the you need to do one or the other, not both?

Something like:

1. Implement the ToModels function in `transformer.go`... - For custom events: - ... - For LogNoteEvents: - ...

elizabethengelman · 2020-04-30T20:22:28Z

transformers/events/DOCUMENTATION.md

+       * the `tend` method is called on the [flip.sol
+       contract](https://github.com/makerdao/dss/blob/master/src/flip.sol#L123), 
+       and its method signature looks like this: `tend(uint,uint,uint)`.
+       * The first four bytes of the Keccak-256 hashed method signature will be 


A small caveat to this is that for custom events the topic[0] will be the full Keccak-256 hashed method signature instead of just the first four bytes - that is special to LogNotes.

I added a bit of clarification here.

rmulhol

Just dropping in to make sure we don't accidentally merge #188 as part of this PR

The issues with formatting are causing a larger diff than I'd like, apologies.

rmulhol

paytonrules force-pushed the vdb-421-update-event-transformer-docs branch from 38ab3be to 2cc8adb Compare April 29, 2020 22:46

paytonrules requested review from elizabethengelman, rmulhol and yaoandrew April 29, 2020 22:47

rmulhol approved these changes Apr 30, 2020

View reviewed changes

elizabethengelman approved these changes Apr 30, 2020

View reviewed changes

Update creating an event transformer doc

63fc5c9

paytonrules force-pushed the vdb-421-update-event-transformer-docs branch from 2cc8adb to 621b02f Compare May 12, 2020 16:29

rmulhol suggested changes May 12, 2020

View reviewed changes

paytonrules added 2 commits May 12, 2020 11:40

Address a ton of feedback, address formatting.

3c8efa9

The issues with formatting are causing a larger diff than I'd like, apologies.

Add some notes around integration testing.

a553198

paytonrules force-pushed the vdb-421-update-event-transformer-docs branch from 621b02f to a553198 Compare May 12, 2020 16:40

rmulhol approved these changes May 12, 2020

View reviewed changes

Merge branch 'staging' into vdb-421-update-event-transformer-docs

b23ec00

paytonrules merged commit 61e6bf6 into staging May 13, 2020

paytonrules deleted the vdb-421-update-event-transformer-docs branch August 26, 2020 15:52


		Simply replace the constants and package names with your transformer.

		1. Finally add your package to the list of transformerExporters in `plugins/transformerExporter.go`. Again alphabetically.

Conversation

paytonrules commented Apr 29, 2020

Uh oh!

rmulhol left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elizabethengelman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rmulhol left a comment

Choose a reason for hiding this comment

Uh oh!

rmulhol left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants