(VDB-1111) flap wards storage transformer#91
Conversation
5638eb6 to
b33c1d1
Compare
b33c1d1 to
b321018
Compare
| } | ||
| headerID int64 | ||
| header = fakes.FakeHeader | ||
| headerID int64 |
There was a problem hiding this comment.
Super minor and not introduced by this PR but I've become a fan of not having a headerID var and instead always accessing that val via header.Id in tests so that we don't get weird failures deriving from headerID != header.Id
| headerID int64 | ||
| header = fakes.FakeHeader | ||
| headerID int64 | ||
| err error |
There was a problem hiding this comment.
Similar thing for err vars - generally prefer to scope these as locally as possible so that we have unique errors in each place they can occur (so that the compiler will tell us if they're unchecked)
| @@ -36,18 +39,18 @@ import ( | |||
| var _ = Describe("Executing the transformer", func() { | |||
There was a problem hiding this comment.
🤔 I wonder if we want to name this configured_transformer_test. Still getting used to converter => transformer, which I think is a win - but the various vat "transformer tests" threw me off for a minute
b321018 to
d3780f3
Compare
| } | ||
|
|
||
| func getUser(keys map[storage.Key]string) (string, error) { | ||
| func GetUser(keys map[storage.Key]string) (string, error) { |
There was a problem hiding this comment.
curious why we're exposing this method outside of this package now 🤔
There was a problem hiding this comment.
Nice catch! I was using it somewhere else, but ended up undoing the change, so this can totally be a private function.
| id SERIAL PRIMARY KEY, | ||
| header_id INTEGER NOT NULL REFERENCES public.headers (id) ON DELETE CASCADE, | ||
| log_id BIGINT NOT NULL REFERENCES public.event_logs (id) ON DELETE CASCADE, | ||
| address_id INTEGER NOT NULL REFERENCES addresses (id) ON DELETE CASCADE, |
There was a problem hiding this comment.
Are we removing the address_id from here (and from vat_deny) because there's only going to be one vat contract per release?
| insertErr := event.PersistModels([]event.InsertionModel{denyModel}, db) | ||
| Expect(insertErr).NotTo(HaveOccurred()) | ||
|
|
||
| key := common.HexToHash("614c9873ec2671d6eb30d7a22b531442a34fc10f8c24a6598ef401fe94d9cb7d") |
There was a problem hiding this comment.
I wonder if we could go through the steps to calculate the key here so it's more explicit about where it came from? Not super important and definitely not merge blocking!
There was a problem hiding this comment.
I like that idea, but in this instance I just grabbed a record from a CSV storage diff. Maybe it would be more thorough to actually calculate the key in the test though 🤔
The code responsible for the calculation is pretty simple:
func GetKeyForMapping(indexOnContract, key string) common.Hash {
keyBytes := common.FromHex(key + indexOnContract)
return crypto.Keccak256Hash(keyBytes)
}
d3780f3 to
59d4e6b
Compare
Includes wards mapping for flip, flap, flop + jug contracts.
Edit: also Vat