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

SRC-14: Owned Proxy #1

Merged
merged 21 commits into from
Jul 15, 2024
Merged

SRC-14: Owned Proxy #1

merged 21 commits into from
Jul 15, 2024

Conversation

K1-R1
Copy link
Member

@K1-R1 K1-R1 commented Jun 30, 2024

Implements an SRC-14 compliant owned proxy contract that uses the new Upgradability lib from sway-libs. As well as basic repo setup

Closes #2

@K1-R1 K1-R1 self-assigned this Jun 30, 2024
@K1-R1 K1-R1 added the enhancement New feature or request label Jul 8, 2024
@K1-R1
Copy link
Member Author

K1-R1 commented Jul 8, 2024

Dependent on FuelLabs/sway-libs#259

@K1-R1 K1-R1 marked this pull request as ready for review July 8, 2024 01:21
@K1-R1 K1-R1 requested review from SwayStar123, bitzoic and a team July 8, 2024 01:22
@bitzoic
Copy link
Member

bitzoic commented Jul 8, 2024

Are you able to pull CI and repo setup functionality to a separate PR to keep things clean?

@K1-R1
Copy link
Member Author

K1-R1 commented Jul 10, 2024

Are you able to pull CI and repo setup functionality to a separate PR to keep things clean?

#4 contains the CI on it's own, but the checks fail as the files don't exist

@K1-R1 K1-R1 mentioned this pull request Jul 10, 2024
Copy link
Member

@bitzoic bitzoic left a comment

Choose a reason for hiding this comment

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

The bytecode roots should be provided so they can be verified against.

Copy link
Member

@DefiCake DefiCake left a comment

Choose a reason for hiding this comment

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

lgtm

not in the scope of this PR, but in the sway libs:

  • CannotReinitialized should be CannotReinitialize?
  • Accidentally default-ing the contained address in State::Initialized (ie State::Initialized(Default::default())) for the owner seems more probable in rust than in other languages. I am not one to complain about these things but it has been raised here and there in audits and in the hackathon. For the minor gas cost and the major time savings in telling people to f-off.. I 'd say it is worth it, but ultimately up to you.

@K1-R1 K1-R1 merged commit 454906c into master Jul 15, 2024
4 checks passed
@K1-R1 K1-R1 deleted the k1-r1-src14-impl branch July 24, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: SRC-14: Owned Proxy
3 participants