AntelopeIO / reference-contracts

Other
9 stars 14 forks source link

Fix fragile powerup_tests/weight_tests so that they can run under Savanna #101

Closed linh2931 closed 3 months ago

linh2931 commented 3 months ago

Under Savanna, powerup weight_tests fails in both NET and CPU weight_ratio tests:

tests/eosio.powerup_tests.cpp(410): fatal error: in "eosio_system_powerup_tests/weight_tests": critical check near(get_state().net.weight_ratio, net, 1) has failed

The root cause is the tests are fragile.

NET and CPU decays are calculated by update_weight in the system contract https://github.com/AntelopeIO/reference-contracts/blob/229dd4f6b358238e34abf9b8ccb26df0b9cbba5c/contracts/eosio.system/src/powerup.cpp#L91-L98

target_timestamp is set in tests for example https://github.com/AntelopeIO/reference-contracts/blob/229dd4f6b358238e34abf9b8ccb26df0b9cbba5c/tests/eosio.powerup_tests.cpp#L382

while initial_timestamp is set in contract https://github.com/AntelopeIO/reference-contracts/blob/229dd4f6b358238e34abf9b8ccb26df0b9cbba5c/contracts/eosio.system/src/powerup.cpp#L323

eosio::current_time_point() is calculated by host function interface::current_time() to be context.control.pending_block_time().time_since_epoch().count().

As pending_block_time() is head_block_time() + 500ms, when head_block_time() is not on the second, pending_block_time().utc_seconds will be the next second; this results in (res.target_timestamp.utc_seconds - res.initial_timestamp.utc_seconds) not equal to exact 10 days in the example, causing rounding errors in tests.

The solution is to use pending_block_time() for target_timestamp.

Note: this does not fix the rent_tests failures. That is tracked by https://github.com/AntelopeIO/reference-contracts/issues/104

Partially resolved https://github.com/AntelopeIO/reference-contracts/issues/91.

Change Description

Deployment Changes

API Changes

Documentation Additions

greg7mdp commented 3 months ago

This does avoid the issue, but I feel that the test is still not fully correct, since it tests results with an accuracy beyond the one provided by the powerup contract (for example [40000.0000 TST != 40000.0001 TST].

Maybe this is OK for now, but I feel that ideally the test should pass regardless of whether the initial block number is odd or even.

linh2931 commented 3 months ago

This does avoid the issue, but I feel that the test is still not fully correct, since it tests results with an accuracy beyond the one provided by the powerup contract (for example [40000.0000 TST != 40000.0001 TST].

Maybe this is OK for now, but I feel that ideally the test should pass regardless of whether the initial block number is odd or even.

Thanks. It is decided to fix weight_test first using the correct pending_block_time for target_timestamp. That test is no longer flaky.

The rent tests failures are now tracked by https://github.com/AntelopeIO/reference-contracts/issues/104