Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Jun 14, 2024

Conversation

linh2931
Copy link
Member

@linh2931 linh2931 commented Jun 12, 2024

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

void update_weight(time_point_sec now, powerup_state_resource& res, int64_t& delta_available) {
if (now >= res.target_timestamp) {
res.weight_ratio = res.target_weight_ratio;
} else {
res.weight_ratio = res.initial_weight_ratio + //
int128_t(res.target_weight_ratio - res.initial_weight_ratio) *
(now.utc_seconds - res.initial_timestamp.utc_seconds) /
(res.target_timestamp.utc_seconds - res.initial_timestamp.utc_seconds);

target_timestamp is set in tests for example

config.net.target_timestamp = time_point_sec(control->head_block_time() + fc::days(10));

while initial_timestamp is set in contract

time_point_sec now = eosio::current_time_point();

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 #104

Partially resolved #91.

Change Description

Deployment Changes

  • Deployment Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

@linh2931 linh2931 requested review from arhag, heifner and greg7mdp June 12, 2024 17:48
@greg7mdp
Copy link
Contributor

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 linh2931 changed the title Fix fragile powerup_tests so that they can run under Savanna Fix fragile powerup_tests//weight_tests so that they can run under Savanna Jun 14, 2024
@linh2931 linh2931 changed the title Fix fragile powerup_tests//weight_tests so that they can run under Savanna Fix fragile powerup_tests/weight_tests so that they can run under Savanna Jun 14, 2024
@linh2931
Copy link
Member Author

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 #104

@linh2931 linh2931 merged commit 3908cfb into main Jun 14, 2024
1 check passed
@linh2931 linh2931 deleted the fix_fragile_powerup_tests branch June 14, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants