-
Notifications
You must be signed in to change notification settings - Fork 187
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
Basic support for create
and create2
#239
Conversation
7111501
to
c74be43
Compare
98e90b5
to
7c166a0
Compare
Codecov Report
@@ Coverage Diff @@
## master #239 +/- ##
==========================================
- Coverage 94.08% 93.94% -0.14%
==========================================
Files 54 54
Lines 3701 3768 +67
==========================================
+ Hits 3482 3540 +58
- Misses 219 228 +9
Continue to review full report at Codecov.
|
pub def create2_foo() -> address: | ||
# `0` is the value being sent and `52` is the address salt | ||
foo: Foo = Foo.create2(0, 52) | ||
return address(foo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we consider giving contracts a special attribute to obtain the address such so that the above could be written as:
pub def create2_foo() -> address:
# `0` is the value being sent and `52` is the address salt
foo: Foo = Foo.create2(0, 52)
return foo.__address__
With the cast, a bug such as the following would be possible that would result in returning the wrong address.
pub def create2_foo() -> address:
# `0` is the value being sent and `52` is the address salt
foo: Foo = Foo.create2(0, 52)
bar: Bar = Bar.create2(0, 11)
return address(bar)
In the example above, we mean to return the address of the newly created foo
but we accidentally return the address of bar
instead. Sure, one can also accidentially return bar.__address__
but it feels to me that having the address accessible as a property removes one layer of error potential. That said, I really dislike the __address__
syntax but wouldn't know a good way to get around the potential for naming conflicts. At least not until we have something like traits that we could bring into scope conditionally to expose special functions/attributes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I wasn't sure how to deal with this.
We can discuss this more in #179 and if we come up with a better idea, it can be changed. For now, I'm inclined to just merge this as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, should be good for now. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Left some comments inline.
7c166a0
to
5fa3083
Compare
What was wrong?
There was no support for contract creation.
How was it fixed?
create
functions to the contract type.To-Do
Update Spec if applicable
Add entry to the release notes (may forgo for trivial changes)
Clean up commit history