Update Upgrade and add upgrade tests #36
Conversation
Leonard-Pat
commented
Jun 20, 2024
•
Leonard-Pat
commented
- Remove gift_status
- Update upgrade methods
- added upgrade tests
Leonard-Pat
changed the title
|
Can you remove the |
done - very weird i dont know where the file came from 😄 |
Co-authored-by: gaetbout <[email protected]>
tests-integration/upgrade.test.ts
Outdated
|
|
||
| it("Upgrade: Too Early", async function () { | ||
| const { factory } = await setupGiftProtocol(); | ||
| const newFactoryClassHash = await manager.declareFixtureContract("GiftFactoryUpgrade"); |
There was a problem hiding this comment.
You don't really need to declare that fixture to test this behavior, but that's nitpick, feel free to just ignore
src/contracts/timelock_upgrade.cairo
Outdated
| @@ -66,12 +72,13 @@ pub mod TimelockUpgradeComponent { | |||
| #[derive(Drop, starknet::Event)] | |||
| struct UpgradeProposed { | |||
| new_implementation: ClassHash, | |||
| ready_at: u64 | |||
| ready_at: u64, | |||
| calldata_hash: felt252 | |||
There was a problem hiding this comment.
Not sure but maybe the event should also emit the calldata_array instead of the hash, or even on top of the hash?
There was a problem hiding this comment.
changed it to just emit the calldata, is that better ?
| await factory.get_calldata_hash().should.eventually.equal(BigInt(hash.computePoseidonHashOnElements(calldata))); | ||
|
|
||
| await manager.increaseTime(MIN_SECURITY_PERIOD + 1n); | ||
| await factory.upgrade(calldata); |
There was a problem hiding this comment.
Actually I'd tend to avoid increaseTime tbh. I just changed this bit from
await manager.increaseTime(MIN_SECURITY_PERIOD + 1n); to await manager.increaseTime(MIN_SECURITY_PERIOD); and it passes...
There was a problem hiding this comment.
all increase time removed
.gitignore
Outdated
| dist | ||
|
|
There was a problem hiding this comment.
there's an empty line here
There was a problem hiding this comment.
but now it shows this (Axel told me to always add an emtpy line but idm)
There was a problem hiding this comment.
i mean the one at line 46, not the one at the bottom
Co-authored-by: gaetbout <[email protected]>
Co-authored-by: gaetbout <[email protected]>
tests-integration/upgrade.test.ts
Outdated
| await expectRevertWithErrorMessage("upgrade/upgrade-too-late", () => factory.upgrade([])); | ||
| }); | ||
|
|
||
| it("Propose: implementation-null", async function () { |
There was a problem hiding this comment.
Nit: rename all "Propose" and use a describe("Propose") instead
tests-integration/upgrade.test.ts
Outdated
| }); | ||
| }); | ||
|
|
||
| it("Cancel Upgrade /w events", async function () { |
