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.

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: connect to already deployed contracts #113

Merged
merged 13 commits into from
Mar 3, 2022

Conversation

iqdecay
Copy link
Contributor

@iqdecay iqdecay commented Feb 25, 2022

Closes #88.
This PR changes the way to instantiate a contract. Since the compiled code is actually not needed
to interact with the contract, we just provide the contract_id.
This PR also creates a "default instantiation" for contracts, mainly for test purposes.

Comment: not sure about some design choices around string or new_default.

@iqdecay iqdecay self-assigned this Feb 25, 2022
@iqdecay iqdecay added abigen enhancement New feature or request labels Feb 25, 2022
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

Looks reasonable. Will defer to @digorithm for final review.

Copy link
Member

@digorithm digorithm left a comment

Choose a reason for hiding this comment

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

Looks good, great job removing the need for passing the compiled to contract around, much better with just the contract id -- just left a tiny request for comment.

fuels-core/src/code_gen/abigen.rs Outdated Show resolved Hide resolved
@digorithm
Copy link
Member

Tiny conflict with master. Ping me when it's resolved so I can take another pass on this PR 😄

Cargo.lock Outdated Show resolved Hide resolved
@iqdecay iqdecay requested a review from digorithm March 3, 2022 17:19
@iqdecay iqdecay requested a review from adlerjohn March 3, 2022 17:52
Copy link
Member

@digorithm digorithm left a comment

Choose a reason for hiding this comment

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

LGTM. Let's keep in mind that this is a breaking change -- we're moving from creating a contract instance with the compiled contract to taking only the contract id. All tests out there will break with this change. It will require minimal changes on the user side, though, similar to the changes in harness.rs in this PR.

@adlerjohn adlerjohn requested a review from digorithm March 3, 2022 17:56
@iqdecay iqdecay merged commit eacebbb into master Mar 3, 2022
@iqdecay iqdecay deleted the vnepveu/feat-connect-existing-contract branch March 3, 2022 18:08
iqdecay added a commit to FuelLabs/sway that referenced this pull request Mar 3, 2022
iqdecay added a commit to FuelLabs/sway that referenced this pull request Mar 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abigen enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Connect contract instance to existing contract
4 participants