search

nspcc-dev / neofs-contract

NeoFS smart-contract
Other
9 stars 17 forks source link

Migration tests are missing something #332

Closed roman-khimov closed 1 year ago

roman-khimov commented 1 year ago

We've got a number of YmFsbG90cw== ("ballots") storage entries in mainnet-3309907-storage.csv which means that migration should fail like this:

Error: deploy contract: script failed (FAULT state) due to an error: at instruction 71 (SYSCALL): failed native call: at instruction 807 (THROW): unhandled exception: "pending vote detected"

However this doesn't happen in migration tests.

roman-khimov commented 1 year ago

Even though #333 allows to proceed with update, we better investigate it at the testing system level, why does it work this way?

cthulhu-rider commented 1 year ago

However this doesn't happen in migration tests.

what do u expect? Test checks that with existing ballots pending vote detected panic occurs. If this panic - test OK, otherwise FAIL

https://github.com/nspcc-dev/neofs-contract/blob/8df64fe444f802b539ff6174cc1ecc4c2e177000/container/migration_test.go#L95-L98

I changed check to CheckUpdateSuccess and got

--- FAIL: TestMigration (0.18s)
=== RUN   TestMigration/mainnet-3309907/container
    basic.go:213: 
...
            Error:          Not equal: 
                            expected: 0x1
                            actual  : 0x2
            Test:           TestMigration/mainnet-3309907/container
            Messages:       at instruction 71 (SYSCALL): failed native call: at instruction 1240 (THROW): unhandled exception: "pending vote detected"
cthulhu-rider commented 1 year ago

I'll add ballot age check to all tests.

roman-khimov commented 1 year ago

I had expected at least a single complete successful run for mainnet. It probably doesn't worth the trouble now, this code will never be executed again.