-
Notifications
You must be signed in to change notification settings - Fork 2
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
Contract calls #74
Contract calls #74
Conversation
c0d4b8f
to
4ba654f
Compare
imports STRING-SYNTAX | ||
|
||
configuration | ||
// TODO: The address should be bytes. |
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.
Is any operation involving the account address or the esdt-id
based on properties that come from them being bytes? If this is just for representation, having them as a string or as integers (like it is done in KEVM) can be simpler to handle.
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.
Normally, the address is a hash value, represented as a sequence of 32 bytes. For our example, I think it's fine to leave it as a String, since it's easier to use in various contexts (e.g. tests), it's more interesting to use descriptive values for addresses, and it should be easy to convert it to bytes whenever we decide to do that.
@@ -24,4 +24,11 @@ module MX-CALL-RESULT-CONFIGURATION | |||
<mx-call-result> .MxCallResult </mx-call-result> | |||
endmodule | |||
|
|||
module MX-CALL-RETV-CONFIGURATION |
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.
This might be a nitpick, but I'd suggest being descriptive with the module names. Reading the module, I understand that RETV
is supposed to mean "Return Values", but sounds more like an acronym.
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.
Fixed
Closes #52