Skip to content

Add versioning to cat contracts [staging]#280

Merged
rmulhol merged 2 commits intostagingfrom
vdb-1599-add-cat-versions
Sep 8, 2020
Merged

Add versioning to cat contracts [staging]#280
rmulhol merged 2 commits intostagingfrom
vdb-1599-add-cat-versions

Conversation

@elizabethengelman
Copy link
Copy Markdown
Contributor

@elizabethengelman elizabethengelman commented Aug 31, 2020

cut from staging

I think that this is a slightly different situation than the last time we added versions to our contracts (auction contracts) since at that time the ABIs were unchanged (I think?), or at least the interface of the contracts we cared about didn't change. Whereas now, we need to account for a new cat ABI. But would love another set of eyes on that.


func auctionFileMethod() string { return getSolidityFunctionSignature(FlipABI(), "file") }
func biteMethod() string { return getSolidityFunctionSignature(CatABI(), "Bite") }
func biteMethod() string { return getSolidityFunctionSignature(Cat100ABI(), "Bite") }
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.

I'm kind of wondering if we want to use Cat110ABI instead here, just to rely on the most recent contract? They should be the same, so it probably doesn't matter too much 🤔

Copy link
Copy Markdown
Contributor

@paytonrules paytonrules Sep 2, 2020

Choose a reason for hiding this comment

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

Why wouldn't we use the latest? Strikes me as odd to use the old one, but I am a Ether-dumb-dumb.

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.

I think using the latest one makes sense - for many of the cat methods, the ABI didn't change, so it doesn't matter too much. But might as well use the most recent one. I'll push up a commit shortly!

@elizabethengelman elizabethengelman marked this pull request as ready for review August 31, 2020 15:33
@elizabethengelman elizabethengelman force-pushed the vdb-1599-add-cat-versions branch from ce006f4 to c270ffd Compare August 31, 2020 16:48
@elizabethengelman elizabethengelman changed the title Add versioning to cat contracts Add versioning to cat contracts [staging] Aug 31, 2020
Copy link
Copy Markdown
Contributor

@paytonrules paytonrules left a comment

Choose a reason for hiding this comment

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

Seems straightforward, mostly follow the compiler stuff, but I don't see where the new version of the contract is used.

It all looks right but I feel like I'm missing something.

@elizabethengelman
Copy link
Copy Markdown
Contributor Author

Yeah, it's not being used in this PR - normally I would have done it as part of #284 but broke out adding the new contract into a separate PR so we could use it for the other transformers that depended on it.

@elizabethengelman elizabethengelman force-pushed the vdb-1599-add-cat-versions branch from 894243e to 06bbb45 Compare September 3, 2020 15:21
@rmulhol rmulhol merged commit c99cf4e into staging Sep 8, 2020
@rmulhol rmulhol deleted the vdb-1599-add-cat-versions branch September 8, 2020 19:14
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.

3 participants