Skip to content

(VDB-1026) Pot storage transformer#79

Merged
rmulhol merged 5 commits intostagingfrom
vdb-1026-pot-storage
Dec 19, 2019
Merged

(VDB-1026) Pot storage transformer#79
rmulhol merged 5 commits intostagingfrom
vdb-1026-pot-storage

Conversation

@gslaughl
Copy link
Copy Markdown
Contributor

I also had to go back and add msg.sender to the join and exit events, since I forgot to include it when I initially wrote those event transformers.

Also I don't have any storage diffs handy, so I fudged the component tests, but it should be pretty easy to go back and amend them if we get ahold of any real diffs.

@gslaughl gslaughl force-pushed the vdb-1026-pot-storage branch 2 times, most recently from d8c0e10 to fb665f4 Compare December 19, 2019 17:09

addrID, addrErr := shared.GetOrCreateAddress("0x87e76b0a50efc20259cafE0530f75aE0e816aaF2", db)
Expect(addrErr).NotTo(HaveOccurred())
Expect(err).NotTo(HaveOccurred())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

return err
}

var insertUserPieDiff diffInserter = func(addressID int64, tx *sqlx.Tx) error {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🥇

queryErr := db.Get(&count, query)
Expect(queryErr).NotTo(HaveOccurred())
Expect(count).To(Equal(1))
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not uses the shared behaviors here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The "persists a record" test is more complex than the shared test suite, because it has to deal with the address FK. But the "duplicate record" test is identical to the one in the shared suite, so maybe it would be worth breaking apart the shared test suite to allow us to extract only the "duplicate record" one in cases like these.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah, that makes sense - would be interested in refactors that make it easier to use the shared tests for fields including a FK, but definitely doesn't need to be a part of this PR 👍

UNION
SELECT addresses.address
FROM maker.pot_exit
LEFT JOIN public.addresses ON pot_exit.msg_sender = addresses.id`)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems legit and follows the pattern we've used, but there should never be an address exiting without a prior join - right? Totally fine with leaving things as is to avoid errors from missed events, just curious

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, totally! I hadn't really thought about that. Hopefully one day we'll have a node so reliable that we won't need to be redundant :)

@rmulhol rmulhol force-pushed the vdb-1026-pot-storage branch from fb665f4 to 18d5d35 Compare December 19, 2019 22:53
@rmulhol rmulhol force-pushed the vdb-1026-pot-storage branch from 18d5d35 to 2d4c9b6 Compare December 19, 2019 23:01
@rmulhol rmulhol merged commit 19ee215 into staging Dec 19, 2019
@rmulhol rmulhol deleted the vdb-1026-pot-storage branch December 19, 2019 23:13
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