diff --git a/src/contracts/gift_factory.cairo b/src/contracts/gift_factory.cairo index 4f4ab4e..5b6e302 100644 --- a/src/contracts/gift_factory.cairo +++ b/src/contracts/gift_factory.cairo @@ -231,8 +231,10 @@ mod GiftFactory { fn perform_upgrade(ref self: ContractState, new_implementation: ClassHash, data: Array) { 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 } } diff --git a/src/contracts/timelock_upgrade.cairo b/src/contracts/timelock_upgrade.cairo index 3c4e4fd..762a68b 100644 --- a/src/contracts/timelock_upgrade.cairo +++ b/src/contracts/timelock_upgrade.cairo @@ -157,11 +157,10 @@ pub mod TimelockUpgradeComponent { } #[generate_trait] - // #[embeddable_as(TimelockUpgradeInternalImpl)] pub impl TimelockUpgradeInternalImpl< - TContractState, impl Ownable: OwnableComponent::HasComponent, +HasComponent + TContractState, +HasComponent > of ITimelockUpgradeInternal { - /// @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) { assert(self.upgrade_lock.read(), 'upgrade/only-during-upgrade'); self.upgrade_lock.write(false); diff --git a/tests-integration/upgrade.test.ts b/tests-integration/upgrade.test.ts index 2035528..250a03e 100644 --- a/tests-integration/upgrade.test.ts +++ b/tests-integration/upgrade.test.ts @@ -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"]; }); @@ -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";