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.

Sign up for GitHub

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

Jump to bottom

feat: add interface validation method for dynamic link functions #245

Merged
merged 7 commits into from
Oct 26, 2022
Merged

feat: add interface validation method for dynamic link functions #245

merged 7 commits into from
Oct 26, 2022

Conversation

loloicci
Copy link
Contributor

@loloicci loloicci commented Sep 29, 2022 •
edited
Loading

Description

Add validate_dynamic_link_interface to deps.api.
contracts/dynamic-caller-contract/src/contract.rs is an example of how to use this.

Types of changes

  • Bug fix (changes which fixes an issue)
  • New feature (changes which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • ETC (build, ci, docs, perf, refactor, style, test)

Checklist

@loloicci loloicci force-pushed the interface-detection branch 2 times, most recently from f1952a9 to 64c9263 Compare September 29, 2022 09:32
@loloicci loloicci mentioned this pull request Sep 29, 2022
Merged
8 tasks
@loloicci loloicci self-assigned this Sep 29, 2022
@loloicci loloicci added the dynamic_link relate the dynamic link call feature label Sep 29, 2022
@loloicci loloicci mentioned this pull request Sep 30, 2022
Merged
5 tasks
zemyblue
zemyblue reviewed Oct 7, 2022
View reviewed changes
Copy link
Member

@zemyblue zemyblue left a comment

Choose a reason for hiding this comment

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

@loloicci, the method that currently implemented interface detection function in this dynamic link is not rustLang specialized, is it? In other words, if smart contract can be developed in other development languages, it can be linked, right?

@loloicci
Copy link
Contributor Author

the method that currently implemented interface detection function in this dynamic link is not rustLang specialized, is it? In other words, if smart contract can be developed in other development languages, it can be linked, right?

Exactry, yes.

@shiki-tak
Copy link
Contributor

As for the design, LGTM
In addition to validating whether an arbitrary function is implemented, it would be better to have the ability to detect whether a particular function is implemented and what kind of token it is, as in ERC 165.

@loloicci loloicci marked this pull request as ready for review October 20, 2022 11:15
@loloicci loloicci marked this pull request as draft October 21, 2022 07:24
@loloicci
ci: upgrade Rust in CI contract building test
045ac42 
This PR upgrade Rust to 1.57.0 in some part of CI.
When CI uses rust 1.51,
error: linking with `cc` failed: exit code: 1
is caused in mac OS v12.
@loloicci loloicci marked this pull request as ready for review October 21, 2022 07:43
@loloicci
Copy link
Contributor Author

it would be better to have the ability to detect whether a particular function is implemented and what kind of token it is, as in ERC 165.

It looks good, but this PR is only for interface validation. So I will leave it on another issue.

zemyblue
zemyblue reviewed Oct 24, 2022
View reviewed changes
Copy link
Member

@zemyblue zemyblue left a comment

Choose a reason for hiding this comment

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

How can I check the code coverage?

contracts/dynamic-caller-contract/src/contract.rs Show resolved Hide resolved
@loloicci
Copy link
Contributor Author

loloicci commented Oct 25, 2022 •
edited
Loading

How can I check the code coverage?

now we do not have features to check code coverage...

shiki-tak
shiki-tak approved these changes Oct 25, 2022
View reviewed changes
Copy link
Contributor

@shiki-tak shiki-tak left a comment

Choose a reason for hiding this comment

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

LGTM

packages/vm/src/dynamic_link.rs Show resolved Hide resolved
@zemyblue
Copy link
Member

How can I check the code coverage?

now we do not have features to check code coverage...

Could you add codecov checking as CI later?

@loloicci loloicci changed the title feat: add interface detection method for dynamic link functions feat: add interface validation method for dynamic link functions Oct 26, 2022
@loloicci loloicci merged commit a611c55 into dynamic_link Oct 26, 2022
@loloicci loloicci deleted the interface-detection branch October 26, 2022 07:21
This was referenced Dec 13, 2022
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shiki-tak shiki-tak shiki-tak approved these changes

@da1suk8 da1suk8 da1suk8 approved these changes

@zemyblue zemyblue zemyblue approved these changes

Assignees

@loloicci loloicci

Labels
dynamic_link relate the dynamic link call feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@loloicci @shiki-tak @zemyblue @da1suk8