-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: add missing self call methods #6
feat: add missing self call methods #6
Conversation
7ef02ee
to
460e0a4
Compare
6196414
to
d6f4b72
Compare
res/mock_evm/src/lib.rs
Outdated
@@ -8,6 +8,7 @@ use near_sdk::{near_bindgen, PanicOnDefault}; | |||
pub mod ft; | |||
mod metadata; | |||
mod out; | |||
mod selfcall; |
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.
Just out of curiosity. What's the point of the such name?
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.
All the modules have names describing the types of methods they define. ft
for FT calls, metadata for view methods. self
is unfortunately not possible since it's a keyword and can't be used as an identifier. Open to better naming.
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.
Maybe private
? The workspace code uses the term SelfCall
so that's why I picked it.
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.
As for me, it's a weird classification from the beginning. I suggest putting all aurora-evm methods into the lib.rs
and methods related to the eth-connector
to move into new created etc_connector.rs
module. ft.rs
and metadata.rs
just remove.
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.
I was basically following the existing convention but I'm happy with your proposed solution as well. I've moved over the self calls to lib.rs and in a separate PR we can move the rest of the stuff over.
I guess I don't know much yet but isn't the eth-connector separate from the engine? Should the two live in the same mock-evm crate?
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.
I think it should because after merging Split NEP-141 we will communicate with aurora-eth-connector
smart contract via cross-contract calls for a while. So, aurora-evm
will be like a proxy for aurora-eth-connector
. It means aurora-evm
will have eth-connector's methods. @mrLSD correct me if I am wrong.
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.
@mandreyel current mock is ok.
@aleksuss no it is not. Eth-connector method after some period of deprecation (like 3 months) will be removed from the Engine. But I'll do things related to the new eth-connector by myself. Don't worry.
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.
@mandreyel I think it can be renamed like "inner_call"
9860b9e
to
c569708
Compare
@mandreyel Pls remove private methods and also update PR description. |
c569708
to
6ada07b
Compare
6ada07b
to
da1305a
Compare
Adds
aurora-engine
contract methods that are missing from the workspace. This PR only deals with the public view methods, others will be added separately.The methods have already been defined in the
SelfCall
enum here, which currently has variants that are never used in the code.Added methods
Note
The
SelfCall
enum variants related to the eth-connector-specific methods (new_eth_connector
,ft_resolve_transfer
,finish_deposit
) have been removed to reflect the changes in the NEP-141 split PR.Please make sure that all the method input/output arguments match those on the engine contract. I tried my best to match them but I'm still lacking some familiarity to be absolutely sure of the changes.
Other changes
Local mocked test cases for these methods have been added. The existing
ft_resolve_transfer
mock-evm stub method has been fixed too.