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

(VDB-366) Add sin mapping to vow storage transformer #51

Merged
merged 1 commit into from
Apr 11, 2019

Conversation

Gslaughl
Copy link
Contributor

@Gslaughl Gslaughl commented Apr 9, 2019

I think this is just about right. Still have a couple questions though

if err != nil {
return nil, err
}
encodedBytes := hexutil.EncodeUint64(uint64(intTimestamp))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This hex stuff is just the first way I found to convert an int to a byte array, not sure if there's a clearer or more idiomatic way of going about it.

Choose a reason for hiding this comment

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

Could you also do something like this: []byte(timestamp) to convert the timestamp string into a byte array?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weirdly no, that doesn't work (I get a different array)... so I guess my initial comment was wrong, I must not just be converting an int to a byte array. I'm gonna dig in a little bit and figure out exactly what's happening in this function

@@ -43,11 +43,15 @@ var FakeHeader = core.Header{
}

func GetFakeHeader(blockNumber int64) core.Header {
return GetFakeHeaderWithTimestamp(fakeTimestamp, blockNumber)
Copy link
Contributor Author

@Gslaughl Gslaughl Apr 9, 2019

Choose a reason for hiding this comment

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

I haven't made this change in vulcanizedb yet, it's part of a PR I'm about to open

this change is now a very small PR in vulcanizedb. I'll copy-paste it into vendors before merging

vulcanize/vulcanizedb#76

It's merged in now

Copy link
Contributor

@rmulhol rmulhol left a comment

Choose a reason for hiding this comment

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

🚀

@Gslaughl Gslaughl force-pushed the vdb-366-vow-storage-update branch 5 times, most recently from 752f9cc to 466422e Compare April 10, 2019 20:40
Copy link

@elizabethengelman elizabethengelman left a comment

Choose a reason for hiding this comment

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

LGTM! :shipit: A couple of small questions, that aren't merge blocking.

db.MustExec("DELETE FROM maker.vow_sin_integer")
db.MustExec("DELETE FROM maker.vow_sump")
db.MustExec("DELETE FROM maker.vow_vat")
db.MustExec("DELETE FROM maker.vow_wait")

Choose a reason for hiding this comment

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

🎉

if err != nil {
return nil, err
}
encodedBytes := hexutil.EncodeUint64(uint64(intTimestamp))

Choose a reason for hiding this comment

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

Could you also do something like this: []byte(timestamp) to convert the timestamp string into a byte array?

return nil
}

func getSinKey(timestamp string) common.Hash {

Choose a reason for hiding this comment

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

Is this string the hex representation of the timestamp? I wonder if it would be helpful to specify that in the var name, like hexTimestamp or something?

@Gslaughl Gslaughl merged commit dccb125 into staging Apr 11, 2019
@Gslaughl Gslaughl deleted the vdb-366-vow-storage-update branch April 11, 2019 14:50
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.

None yet

3 participants