-
Notifications
You must be signed in to change notification settings - Fork 10
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
Upgrade the repo to Cairo v2.6 #40
Conversation
Upgrade to Cairo v2.6.3 Upgrade to OpenZeppelin v0.10.0 Migrate most contracts except for presets to components (WiP)
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.
Minor comments but overall lgtm 💪
congrats on passing all the tests!
@@ -292,15 +365,18 @@ mod ERC3525 { | |||
|
|||
// [Effect] Update total value | |||
let slot = self.slot_of(token_id); | |||
let total = self._erc3525_total_value.read(slot); | |||
let _total = self._erc3525_total_value.read(slot); |
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.
This line might be removed I guess.
// [Effect] Merge token values | ||
let erc721_comp = get_dep_component!(@self, ERC721); | ||
let mut index = 0; | ||
loop { |
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.
This could be a while
loop
impl ExternalImpl of super::IExternal<ContractState> { | ||
fn total_value(self: @ContractState, slot: u256) -> u256 { | ||
let unsafe_state = ERC3525::unsafe_new_contract_state(); | ||
ERC3525::InternalImpl::_total_value(@unsafe_state, slot) | ||
self.erc3525._total_value(slot) | ||
} | ||
|
||
fn mint(ref self: ContractState, to: ContractAddress, slot: u256, value: u256) -> u256 { |
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.
Could this have been a IERC3525MintableComponent
component too? ?
Things already done:
ERC3525
module and its extensions to componentsname
andsymbol
methods fromfelt252
toByteArray
due to changes in OpenZeppelin ERC721 contractset_token_uri
withset_base_uri
for the same reasonThings left:
Will fix #39