PSE upgrade integration test#40
Conversation
miladz68
left a comment
There was a problem hiding this comment.
@miladz68 reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @masihyeganeh and @ysv)
integration-tests/upgrade/pse_initial_distribution.go line 53 at r1 (raw file):
}) requireT.NoError(err) pid.clearingAccountBefore[clearingAccount] = *balanceResp.Balance
all clearing balances before must be zero, there is not point tracking it. just assert that they are zero.
integration-tests/upgrade/pse_initial_distribution.go line 207 at r1 (raw file):
currentTime := time.Unix(int64(period.Timestamp), 0).UTC() requireT.Equal(1, currentTime.Day(),
this assertion will not be true anymore.
TxCorpi0x
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @masihyeganeh, @miladz68, and @ysv)
integration-tests/upgrade/pse_initial_distribution.go line 53 at r1 (raw file):
Previously, miladz68 (milad) wrote…
all clearing balances before must be zero, there is not point tracking it. just assert that they are zero.
The module accounts have value in the beginning, they are created by znet, and this is an integration test.
integration-tests/upgrade/pse_initial_distribution.go line 207 at r1 (raw file):
Previously, miladz68 (milad) wrote…
this assertion will not be true anymore.
NEw logic merged and Fixed.
miladz68
left a comment
There was a problem hiding this comment.
@miladz68 reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @masihyeganeh, @TxCorpi0x, and @ysv)
integration-tests/upgrade/pse_initial_distribution.go line 53 at r1 (raw file):
Previously, TxCorpi0x wrote…
The module accounts have value in the beginning, they are created by znet, and this is an integration test.
before upgrade, we start with cored-v5.0.0 binary.
TxCorpi0x
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @masihyeganeh, @miladz68, and @ysv)
integration-tests/upgrade/pse_initial_distribution.go line 53 at r1 (raw file):
Previously, miladz68 (milad) wrote…
before upgrade, we start with cored-v5.0.0 binary.
Done
miladz68
left a comment
There was a problem hiding this comment.
@miladz68 reviewed 1 of 1 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @masihyeganeh and @ysv)
ysv
left a comment
There was a problem hiding this comment.
@ysv reviewed 1 of 2 files at r1, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @masihyeganeh and @TxCorpi0x)
integration-tests/upgrade/pse_initial_distribution.go line 148 at r3 (raw file):
// 3. Amount already distributed alreadyDistributed := monthlyAmount.MulRaw(int64(processedDistributions))
but this will always be eq to 0 right after upgrade, isn't it ?
Code quote:
// 3. Amount already distributed
alreadyDistributed := monthlyAmount.MulRaw(int64(processedDistributions))integration-tests/upgrade/pse_initial_distribution.go line 182 at r3 (raw file):
t.Logf("Total actual balance: %s", totalActualBalance) return allocations
do we really need to return this if we never use it ?
integration-tests/upgrade/pse_initial_distribution.go line 200 at r3 (raw file):
t.Logf("Distribution schedule created with %d periods", len(schedule)) return schedule
do we really need to return this if we never use it ?
integration-tests/upgrade/pse_initial_distribution.go line 220 at r3 (raw file):
// Verify all distributions are on the same day of the month at 12:00:00 GMT expectedDay := firstDistTime.Day()
will this will fail if we run test on 29-31 day of the month ?
TxCorpi0x
left a comment
There was a problem hiding this comment.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @masihyeganeh and @ysv)
integration-tests/upgrade/pse_initial_distribution.go line 148 at r3 (raw file):
Previously, ysv (Yaroslav Savchuk) wrote…
but this will always be eq to 0 right after upgrade, isn't it ?
You are right, removed.
integration-tests/upgrade/pse_initial_distribution.go line 182 at r3 (raw file):
Previously, ysv (Yaroslav Savchuk) wrote…
do we really need to return this if we never use it ?
Please check the lines 74-78 in After, it has been used there.
integration-tests/upgrade/pse_initial_distribution.go line 200 at r3 (raw file):
Previously, ysv (Yaroslav Savchuk) wrote…
do we really need to return this if we never use it ?
Please check the lines 74-78 in After, it has been used there.
integration-tests/upgrade/pse_initial_distribution.go line 220 at r3 (raw file):
Previously, ysv (Yaroslav Savchuk) wrote…
will this will fail if we run test on 29-31 day of the month ?
No, the pse initialization caps the days to 28, so after upgrade, event if the day is 29, 30, or 31st, it won't fail
* PSE start time on upgrade time day start UTC * Set distribution schedules duration as 30days * Change start time to 12:00 GMT on 28th of month * Rename period to month in PSE init
* added some wait time to pse upgrade test * fix a comment * changed the wait logic * fix linter * Merge branch 'master' into milad/add-wait-pse-upgrade-test
* ClI Query for clearing accounts and schedule * PSE queries integration tests * Merge branch 'master' into mehdi/pse-query-cli * Merge branch 'master' into mehdi/pse-query-cli * Resolve comments * Lint fix * Merge branch 'master' into mehdi/pse-query-cli * Fix conflicts * Merge branch 'master' into mehdi/pse-query-cli
b79b9f9 to
29e09ad
Compare
ysv
left a comment
There was a problem hiding this comment.
@ysv reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @masihyeganeh and @TxCorpi0x)
integration-tests/upgrade/pse_initial_distribution.go line 148 at r3 (raw file):
Previously, TxCorpi0x wrote…
You are right, removed.
still visible in the last committed version
ysv
left a comment
There was a problem hiding this comment.
@ysv reviewed 4 of 4 files at r5, all commit messages.
Reviewable status:
TxCorpi0x
left a comment
There was a problem hiding this comment.
Reviewable status:
integration-tests/upgrade/pse_initial_distribution.go line 148 at r3 (raw file):
Previously, ysv (Yaroslav Savchuk) wrote…
still visible in the last committed version
I was force pushed to remove an older commit, rebased with master with the other PR
ysv
left a comment
There was a problem hiding this comment.
Reviewable status:
miladz68
left a comment
There was a problem hiding this comment.
@miladz68 reviewed 4 of 4 files at r5, all commit messages.
Reviewable status:
Description
This pull request updates the PSE initial fund allocation and distribution schedule to start one month after the v6 upgrade, rather than on the upgrade day itself. It also introduces a new integration test to verify the correctness of the initial distribution logic and updates documentation to reflect the new schedule. The most important changes are grouped below.
Core logic changes:
app/upgrade/v6/pse_init.go.Test updates:
app/upgrade/v6/pse_init_test.gohave been updated to expect the first distribution to occur one month after the upgrade, including timestamp calculations, assertions, and test descriptions.Integration testing:
integration-tests/upgrade/pse_initial_distribution.gohas been added to verify initial fund allocations, distribution schedule, clearing account mappings, and balances after upgrade. This test is now included in the upgrade test suite.Documentation updates:
x/pse/spec/README.mdhas been updated to state that distributions begin one month after the upgrade, not on the upgrade day.Reviewers checklist:
Authors checklist
This change is