Skip to content

Commit

Permalink
improve upgrade mechanism
Browse files Browse the repository at this point in the history
  • Loading branch information
sgc-code committed Jul 9, 2024
1 parent 9a92001 commit 52b346f
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 5 deletions.
6 changes: 4 additions & 2 deletions src/contracts/gift_factory.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,10 @@ mod GiftFactory {
fn perform_upgrade(ref self: ContractState, new_implementation: ClassHash, data: Array<felt252>) {
self.timelock_upgrade.assert_and_reset_lock();
// This should do some sanity checks and ensure that the new implementation is a valid implementation,
// then it will replace_class_syscall
panic_with_felt252('gift-fac/downgrade-not-allowed');
// then it can call replace_class_syscall and emit the UpgradeExecuted event
panic_with_felt252(
'gift-fac/downgrade-not-allowed'
); // since this is the first version nobody should be calling this method
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/contracts/timelock_upgrade.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,10 @@ pub mod TimelockUpgradeComponent {
}

#[generate_trait]
// #[embeddable_as(TimelockUpgradeInternalImpl)]
pub impl TimelockUpgradeInternalImpl<
TContractState, impl Ownable: OwnableComponent::HasComponent<TContractState>, +HasComponent<TContractState>
TContractState, +HasComponent<TContractState>
> of ITimelockUpgradeInternal<TContractState> {
/// @notice Should be called but the `perform_upgrade` method to make sure this method can only by called when upgrading
/// @notice Should be called by the `perform_upgrade` method to make sure this method can only by called when upgrading
fn assert_and_reset_lock(ref self: ComponentState<TContractState>) {
assert(self.upgrade_lock.read(), 'upgrade/only-during-upgrade');
self.upgrade_lock.write(false);
Expand Down
13 changes: 13 additions & 0 deletions tests-integration/upgrade.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ describe("Test Factory Upgrade", function () {
newFactory.connect(deployer);
await newFactory.get_num().should.eventually.equal(1n);

// we can't call the perform_upgrade method directly
await expectRevertWithErrorMessage("upgrade/only-during-upgrade", () =>
factory.perform_upgrade(newFactoryClassHash, []),
);

// clear deployment cache
delete protocolCache["GiftFactory"];
});
Expand Down Expand Up @@ -79,6 +84,14 @@ describe("Test Factory Upgrade", function () {
await expectRevertWithErrorMessage("Caller is not the owner", () => factory.upgrade([]));
});

it("no calls to perform_upgrade", async function () {
const { factory } = await setupGiftProtocol();
const newFactoryClassHash = "0x1";
await expectRevertWithErrorMessage("upgrade/only-during-upgrade", () =>
factory.perform_upgrade(newFactoryClassHash, []),
);
});

it("Invalid Calldata", async function () {
const { factory } = await setupGiftProtocol();
const newFactoryClassHash = "0x1";
Expand Down

0 comments on commit 52b346f

Please sign in to comment.