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/implementation authority #70

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Conversation

EgeCaner
Copy link
Collaborator

No description provided.

@EgeCaner EgeCaner requested a review from julio4 January 15, 2025 11:04
crates/factory/src/iimplementation_authority.cairo Outdated Show resolved Hide resolved
#[storage]
struct Storage {
current_version: Version,
///TODO: Consider making this enumerable to enumerate all available implementations
Copy link
Member

Choose a reason for hiding this comment

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

Making Version Enumerable?
There's not a finite set of versions so I think struct type is better

Copy link
Member

Choose a reason for hiding this comment

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

Maybe add comment //Map<VersionPacked, TREXImplementationsStore> for readability

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My comment was not sufficient to express my intention, I was trying to meant using EnumerableMap to list all the available versions stored in the contract. Currently only way to fetch all versions is querying through events

Copy link
Member

Choose a reason for hiding this comment

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

Ok I agree it could be useful. This would potentially requires using an unique sequential identifier for each version to be able to quickly iterate over all existing versions

self._use_trex_version(version);
}

fn upgrade_trex_suite(ref self: ContractState, token: ContractAddress, version: Version) {
Copy link
Member

Choose a reason for hiding this comment

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

so upgrade_trex_suite replace changeImplementationAuthority right

Copy link
Member

Choose a reason for hiding this comment

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

So there's not a single reference contract and potential auxiliary authority contracts?
Instead, there's a single authority contract that keep track of the current version and the list of possible versions.
The owner can call upgrade_trex_suite to any versions

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes exactly, as far as I see auxiliary authorities can only use versions stored in reference contract. This behaviour mimics it without having multiple contracts. I see no need in having multiple contracts, unlike solidity we dont have proxies that queries implementation authority with same selector like get_ctr_implementation() etc... on each call

Copy link
Member

Choose a reason for hiding this comment

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

yes, LGTM!

higher order bits should be zeros at this point so it should be okay to not mask

Co-authored-by: Julio <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implementation Authority + IA Factory + propogation of the logic where it requires
2 participants