update contracts and tests for cairo 2.5.x #363
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
augustbleeds
temporarily deployed
to
integration
GitHub Actions
Inactive
augustbleeds
temporarily deployed
to
integration
GitHub Actions
Inactive
augustbleeds
had a problem deploying
to
integration
GitHub Actions
Failure
chris-de-leon-cll
previously approved these changes
Feb 23, 2024
archseer
reviewed
Feb 26, 2024
contracts/src/account.cairo
Outdated
| @@ -1,57 +1,233 @@ | |||
| // copied from https://github.com/OpenZeppelin/cairo-contracts/blob/v0.8.1/src/presets/account.cairo | |||
| // copied from https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.9.0/src/access/ownable/ownable.cairo | |||
There was a problem hiding this comment.
Let's use a sha since tags can change
| @@ -107,6 +107,9 @@ trait Billing<TContractState> { | |||
| fn link_available_for_payment( | |||
| self: @TContractState | |||
| ) -> (bool, u128); // (is negative, absolute difference) | |||
| fn set_link_token( | |||
| set_contract_address(s1); | ||
| multisig.confirm_transaction(0); | ||
|
|
||
| // Signer 2 confirms the transaction | ||
| set_caller_address(s2); |
There was a problem hiding this comment.
Hmm was this API removed?
There was a problem hiding this comment.
nah, it wasn't removed. we weren't using it the correct way. set_contract_address should only be used if the test contract is deployed in the testing suite. Otherwise we should use set_caller_address.
I noticed when testing that there is some undefined behavior if you do both
| @@ -102,7 +103,7 @@ mod LinkToken { | |||
| } | |||
There was a problem hiding this comment.
There's a TODO to expose the minter method a couple lines above, can we take care of that here too?
augustbleeds
temporarily deployed
to
integration
GitHub Actions
Inactive
augustbleeds
had a problem deploying
to
integration
GitHub Actions
Failure
augustbleeds
temporarily deployed
to
integration
GitHub Actions
Inactive
archseer
approved these changes
Feb 27, 2024
archseer
requested changes
Feb 27, 2024
archseer
requested changes
Feb 27, 2024
contracts/src/account.cairo
Outdated
|
|
||
| /// # Account Preset | ||
| /// # Ownable Component |
There was a problem hiding this comment.
You seem to have replaced the account contract with the ownable contract?
archseer
had a problem deploying
to
integration
GitHub Actions
Failure
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Used cairo 2.5.4, the last version of 2.5.x
Changes include
pending_owneris used instead ofproposed_ownerand there is no 0 address check during a pending_owner proposal. This is fine because there will be never anyone who can confirm the ownership (no one will be 0 address) so the original owner will still be the owner and can try again.