diff --git a/framework/libra-framework/sources/block.move b/framework/libra-framework/sources/block.move index ce647bf2c..215ce99c0 100644 --- a/framework/libra-framework/sources/block.move +++ b/framework/libra-framework/sources/block.move @@ -12,6 +12,7 @@ module diem_framework::block { use diem_framework::system_addresses; use diem_framework::timestamp; // use diem_std::debug::print; + use ol_framework::testnet; //////// 0L //////// use ol_framework::epoch_boundary; @@ -153,15 +154,20 @@ module diem_framework::block { stake::update_performance_statistics(proposer_index, failed_proposer_indices); state_storage::on_new_block(reconfiguration::current_epoch()); - if (timestamp - reconfiguration::last_reconfiguration_time() >= block_metadata_ref.epoch_interval) { - epoch_boundary::epoch_boundary( - &vm, - reconfiguration::get_current_epoch(), - round - ); - // TODO check this order - reconfiguration::reconfigure(); - + if (timestamp - reconfiguration::last_reconfiguration_time() >= + block_metadata_ref.epoch_interval) { + // if we are in test mode, have the VM do the reconfiguration + if (testnet::is_not_mainnet()) { + epoch_boundary::epoch_boundary( + &vm, + reconfiguration::get_current_epoch(), + round + ); + } else { + // flip the epoch bit, so that the epoch + // boundary can be called by any transaction + epoch_boundary::enable_epoch_trigger(&vm, reconfiguration::get_current_epoch()); + } }; } diff --git a/framework/libra-framework/sources/modified_source/diem_governance.move b/framework/libra-framework/sources/modified_source/diem_governance.move index 958497862..4d4314876 100644 --- a/framework/libra-framework/sources/modified_source/diem_governance.move +++ b/framework/libra-framework/sources/modified_source/diem_governance.move @@ -20,6 +20,7 @@ module diem_framework::diem_governance { use diem_framework::voting; use ol_framework::libra_coin::LibraCoin; + use ol_framework::epoch_boundary; // use diem_std::debug::print; @@ -584,19 +585,23 @@ module diem_framework::diem_governance { // assert!(libra_coin::has_mint_capability(core_resources), error::unauthenticated(EUNAUTHORIZED)); // get_signer(signer_address) // } + /// Any end user can triger epoch/boundary and reconfiguration + /// as long as the VM set the BoundaryBit to true. + /// We do this because we don't want the VM calling complex + /// logic itself. Any abort would cause a halt. + /// On the other hand, a user can call the function once the VM + /// decides the epoch can change. Any error will just cause the + /// user's transaction to abort, but the chain will continue. + /// Whatever fix is needed can be done online with on-chain governance. + public fun trigger_epoch(_sig: &signer) acquires GovernanceResponsbility { // doesn't need a signer + let framework_signer = get_signer(@0x1); + let _ = epoch_boundary::can_trigger(); // will abort if false + epoch_boundary::trigger_epoch(&framework_signer); + } + /// Return the voting power a stake pool has with respect to governance proposals. fun get_voting_power(_pool_address: address): u64 { - // let allow_validator_set_change = staking_config::get_allow_validator_set_change(&staking_config::get()); - // if (allow_validator_set_change) { - // let (active, _, pending_active, pending_inactive) = stake::get_stake(pool_address); - // // We calculate the voting power as total non-inactive stakes of the pool. Even if the validator is not in the - // // active validator set, as long as they have a lockup (separately checked in create_proposal and voting), their - // // stake would still count in their voting power for governance proposals. - // active + pending_active + pending_inactive - // } else { - // stake::get_current_epoch_voting_power(pool_address) - // } 1 } diff --git a/framework/libra-framework/sources/modified_source/reconfiguration.move b/framework/libra-framework/sources/modified_source/reconfiguration.move index a6b9a2c1f..4d2921e50 100644 --- a/framework/libra-framework/sources/modified_source/reconfiguration.move +++ b/framework/libra-framework/sources/modified_source/reconfiguration.move @@ -17,8 +17,9 @@ module diem_framework::reconfiguration { // use diem_std::debug::print; friend diem_framework::diem_governance; - friend diem_framework::block; + friend diem_framework::epoch_boundary; friend diem_framework::consensus_config; + // TODO: check if these are necessary friend diem_framework::execution_config; friend diem_framework::gas_schedule; friend diem_framework::genesis; @@ -105,6 +106,7 @@ module diem_framework::reconfiguration { /// Signal validators to start using new configuration. Must be called from friend config modules. public(friend) fun reconfigure() acquires Configuration { + // Do not do anything if genesis has not finished. if (chain_status::is_genesis() || timestamp::now_microseconds() == 0) { return diff --git a/framework/libra-framework/sources/ol_sources/epoch_boundary.move b/framework/libra-framework/sources/ol_sources/epoch_boundary.move index 61521c28c..256f58ef8 100644 --- a/framework/libra-framework/sources/ol_sources/epoch_boundary.move +++ b/framework/libra-framework/sources/ol_sources/epoch_boundary.move @@ -15,6 +15,8 @@ module diem_framework::epoch_boundary { use ol_framework::infra_escrow; use ol_framework::oracle; use ol_framework::ol_account; + use ol_framework::testnet; + use diem_framework::reconfiguration; use diem_framework::transaction_fee; use diem_framework::system_addresses; use diem_framework::coin::{Self, Coin}; @@ -24,15 +26,32 @@ module diem_framework::epoch_boundary { use diem_std::debug::print; - friend diem_framework::block; + friend diem_framework::diem_governance; + friend diem_framework::block; // for testnet only + //////// ERRORS //////// + /// The transaction fee coin has not been initialized + const ETX_FEES_NOT_INITIALIZED: u64 = 0; + + /// Epoch trigger only implemented on mainnet + const ETRIGGER_EPOCH_MAINNET: u64 = 1; + + /// Epoch is not ready for reconfiguration + const ETRIGGER_NOT_READY: u64 = 2; + + /// Epoch number mismat + const ENOT_SAME_EPOCH: u64 = 3; + + /////// Constants //////// /// how many PoF baseline rewards to we set aside for the miners. /// equivalent reward of one seats of the validator set const ORACLE_PROVIDERS_SEATS: u64 = 1; - /// The transaction fee coin has not been initialized - const ETX_FEES_NOT_INITIALIZED: u64 = 0; - + // the VM can set the boundary bit to allow reconfiguration + struct BoundaryBit has key { + ready: bool, + closing_epoch: u64, + } // I just checked in, to see what condition my condition was in. struct BoundaryStatus has key, drop { @@ -98,6 +117,13 @@ module diem_framework::epoch_boundary { public fun initialize(framework: &signer) { if (!exists(@ol_framework)){ move_to(framework, reset()); + }; + + if (!exists(@ol_framework)) { + move_to(framework, BoundaryBit { + ready: false, + closing_epoch: 0, + }); } } @@ -160,6 +186,51 @@ module diem_framework::epoch_boundary { } + /// flip the bit to allow the epoch to be reconfigured on any transaction + public(friend) fun enable_epoch_trigger(vm_signer: &signer, closing_epoch: u64) + acquires BoundaryBit { + + if (!exists(@ol_framework)) { + move_to(vm_signer, BoundaryBit { + closing_epoch: closing_epoch, + ready: true, + }) + } else { + let state = borrow_global_mut(@ol_framework); + state.closing_epoch = closing_epoch; + state.ready = true; + } + } + /// Once epoch boundary time has passed, and the BoundaryBit set to true + /// any user can trigger the epoch boundary. + /// Why do this? It's preferable that the VM never trigger any function. + /// An abort by the VM will cause a network halt. The same abort, if called + /// by a user, would not cause a halt. + public(friend) fun trigger_epoch(root: &signer) acquires BoundaryBit, + BoundaryStatus { + // must be mainnet + assert!(!testnet::is_not_mainnet(), ETRIGGER_EPOCH_MAINNET); + // must get root permission from governance.move + system_addresses::assert_ol(root); + let _ = can_trigger(); // will abort if false + + // update the state and flip the Bit + let state = borrow_global_mut(@ol_framework); + state.ready = false; + + epoch_boundary(root, state.closing_epoch, 0); + } + + #[view] + /// check to see if the epoch Boundary Bit is true + public fun can_trigger(): bool acquires BoundaryBit { + let state = borrow_global_mut(@ol_framework); + assert!(state.ready, ETRIGGER_NOT_READY); + assert!(state.closing_epoch == reconfiguration::get_current_epoch(), + ENOT_SAME_EPOCH); + true + } + // Contains all of 0L's business logic for end of epoch. // This removed business logic from reconfiguration.move // and prevents dependency cycling. @@ -232,11 +303,10 @@ module diem_framework::epoch_boundary { status.pof_thermo_increase = t_increase; status.pof_thermo_amount = t_amount; - print(&string::utf8(b"EPOCH BOUNDARY END")); - print(status); + print(borrow_global(@ol_framework)); + reconfiguration::reconfigure(); } - // TODO: instrument all of this /// withdraw coins and settle accounts for validators and oracles /// returns the list of compliant_vals fun settle_accounts(root: &signer, compliant_vals: vector
, status: &mut BoundaryStatus): vector
{ @@ -404,11 +474,24 @@ module diem_framework::epoch_boundary { } #[test_only] - public fun ol_reconfigure_for_test(vm: &signer, closing_epoch: u64, epoch_round: u64) acquires BoundaryStatus { + public fun ol_reconfigure_for_test(vm: &signer, closing_epoch: u64, + epoch_round: u64) acquires BoundaryStatus { use diem_framework::system_addresses; system_addresses::assert_ol(vm); epoch_boundary(vm, closing_epoch, epoch_round); } + #[test_only] + public fun test_set_boundary_ready(vm: &signer, closing_epoch: u64) acquires + BoundaryBit { + // don't check for "testnet" here, otherwise we can't test the production settings + enable_epoch_trigger(vm, closing_epoch); + } + + #[test_only] + public fun test_trigger(vm: &signer) acquires BoundaryStatus, BoundaryBit { + // don't check for "testnet" here, otherwise we can't test the production settings + trigger_epoch(vm); + } } diff --git a/framework/libra-framework/sources/ol_sources/mock.move b/framework/libra-framework/sources/ol_sources/mock.move index fe1464912..a5e4905a6 100644 --- a/framework/libra-framework/sources/ol_sources/mock.move +++ b/framework/libra-framework/sources/ol_sources/mock.move @@ -279,11 +279,19 @@ module ol_framework::mock { // the reconfiguration module must run last, since no other // transactions or operations can happen after the reconfig. public fun trigger_epoch(root: &signer) { - let old_epoch = epoch_helper::get_current_epoch(); - epoch_boundary::ol_reconfigure_for_test(root, reconfiguration::get_current_epoch(), block::get_current_block_height()); + let old_epoch = reconfiguration::get_current_epoch(); timestamp::fast_forward_seconds(EPOCH_DURATION); - reconfiguration::reconfigure_for_test(); - assert!(epoch_helper::get_current_epoch() > old_epoch, EDID_NOT_ADVANCE_EPOCH); + + epoch_boundary::ol_reconfigure_for_test(root, old_epoch, + block::get_current_block_height()); + + // always advance + assert!(reconfiguration::get_current_epoch() > old_epoch, + EDID_NOT_ADVANCE_EPOCH); + + // epoch helper should always be in sync + assert!(reconfiguration::get_current_epoch() == + epoch_helper::get_current_epoch(), 666); } // // function to deposit into network fee account diff --git a/framework/libra-framework/sources/ol_sources/tests/_meta_epoch.test.move b/framework/libra-framework/sources/ol_sources/tests/_meta_epoch.test.move index d7092e772..a09f0493d 100644 --- a/framework/libra-framework/sources/ol_sources/tests/_meta_epoch.test.move +++ b/framework/libra-framework/sources/ol_sources/tests/_meta_epoch.test.move @@ -3,34 +3,29 @@ /// tests for external apis, and where a dependency cycle with genesis is created. module ol_framework::test_meta { use diem_framework::reconfiguration; - // use diem_framework::timestamp; - use ol_framework::mock; - // can we trigger a reconfiguration and get to a new epoch? - // see also mock::trigger_epoch() and meta_epoch() test - - // #[test(root = @ol_framework)] - // fun test_reconfigure_custom(root: signer) { - - // mock::ol_test_genesis(&root); - // // NOTE: There was no genesis END event here. - // // Which means we need to use test_helper_increment_epoch_dont_reconfigure - - // let a = reconfiguration::get_current_epoch(); - - // // create a new epoch - // stake::end_epoch(); + #[test(root = @ol_framework)] + fun mock_epochs_advance(root: &signer) { + // Scenario: Testing that if an action expires voting cannot be done. - // reconfiguration::test_helper_increment_epoch_dont_reconfigure(1); + let _vals = mock::genesis_n_vals(root, 2); + // we are at epoch 0 + let epoch = reconfiguration::get_current_epoch(); + assert!(epoch == 0, 7357001); + mock::trigger_epoch(root); // epoch 1 + let epoch = reconfiguration::get_current_epoch(); + assert!(epoch == 1, 7357002); - // let b = reconfiguration::get_current_epoch(); + mock::trigger_epoch(root); // epoch 2 + mock::trigger_epoch(root); // epoch 3 + mock::trigger_epoch(root); // epoch 4 + let epoch = reconfiguration::get_current_epoch(); + assert!(epoch == 4, 7357003); - // assert!(a == 0, 10001); - // assert!(b == 1, 10002); + } - // } #[test(root = @ol_framework)] // can we trigger a reconfiguration and get to a new epoch? @@ -39,7 +34,6 @@ module ol_framework::test_meta { mock::genesis_n_vals(&root, 4); let a = reconfiguration::get_current_epoch(); - // create a new epoch reconfiguration::reconfigure_for_test(); @@ -49,17 +43,4 @@ module ol_framework::test_meta { assert!(b == 1, 10002); } - // #[test(root = @ol_framework)] - // fun test_reconfigure_mock_trigger(root: signer) { - // mock::ol_test_genesis(&root); - // // mock::ol_initialize_coin(&root); - // let a = reconfiguration::get_current_epoch(); - - // mock::trigger_epoch(&root); - // let b = reconfiguration::get_current_epoch(); - - // assert!(a == 0, 10001); - // assert!(b == 1, 10002); - - // } } diff --git a/framework/libra-framework/sources/ol_sources/tests/boundary.test.move b/framework/libra-framework/sources/ol_sources/tests/boundary.test.move index 8959fd2bb..a6beb88f4 100644 --- a/framework/libra-framework/sources/ol_sources/tests/boundary.test.move +++ b/framework/libra-framework/sources/ol_sources/tests/boundary.test.move @@ -14,6 +14,8 @@ module ol_framework::test_boundary { use ol_framework::epoch_boundary; use ol_framework::ol_account; use diem_framework::reconfiguration; + use diem_framework::timestamp; + use diem_framework::diem_governance; // use diem_std::debug::print; @@ -44,10 +46,8 @@ module ol_framework::test_boundary { fun e2e_boundary_happy(root: signer) { let _vals = common_test_setup(&root); - mock::trigger_epoch(&root); - let _vals_post = stake::get_current_validators(); assert!(epoch_boundary::get_reconfig_success(), 7357001); @@ -151,4 +151,63 @@ module ol_framework::test_boundary { assert!(vector::length(&epoch_boundary::get_auction_winners()) == vector::length(&qualified_bidders) , 7357003); assert!(epoch_boundary::get_reconfig_success(), 7357001); } + + #[test(root = @ol_framework, marlon = @0x12345)] + fun epoch_any_address_trigger(root: &signer, marlon: &signer) { + common_test_setup(root); + // testing mainnet, so change the chainid + testnet::unset(root); + // test setup advances to epoch #2 + let epoch = reconfiguration::get_current_epoch(); + assert!(epoch == 2, 7357001); + epoch_boundary::test_set_boundary_ready(root, epoch); + + // test the APIs as root + timestamp::fast_forward_seconds(1); // needed for reconfig + epoch_boundary::test_trigger(root); + let epoch = reconfiguration::get_current_epoch(); + assert!(epoch == 3, 7357002); + + // scenario: marlon has no privileges + // but he can still trigger an epoch if it is enabled + + epoch_boundary::test_set_boundary_ready(root, epoch); + timestamp::fast_forward_seconds(1); // needed for reconfig + diem_governance::trigger_epoch(marlon); + let epoch = reconfiguration::get_current_epoch(); + assert!(epoch == 4, 7357003); + } + + + #[test(root = @ol_framework, marlon = @0x12345)] + #[expected_failure(abort_code = 2, location = 0x1::epoch_boundary)] + fun epoch_any_address_aborts(root: &signer, marlon: &signer) { + common_test_setup(root); + // testing mainnet, so change the chainid + testnet::unset(root); + // test setup advances to epoch #2 + let epoch = reconfiguration::get_current_epoch(); + assert!(epoch == 2, 7357001); + epoch_boundary::test_set_boundary_ready(root, epoch); + + // test the APIs as root + timestamp::fast_forward_seconds(1); // needed for reconfig + epoch_boundary::test_trigger(root); + let epoch = reconfiguration::get_current_epoch(); + assert!(epoch == 3, 7357002); + + // scenario: marlon has no privileges + // if the epoch Boundary Bit is not set, then abort + // Note: the Epoch will not change, but we can't assert + // in the test + + ////// NOTE: not calling this`test_set_boundary_ready` + // epoch_boundary::test_set_boundary_ready(root, epoch); + ////// + + timestamp::fast_forward_seconds(1); + diem_governance::trigger_epoch(marlon); + // aborts + } + } diff --git a/framework/libra-framework/sources/ol_sources/tests/vote_lib/multi_action.test.move b/framework/libra-framework/sources/ol_sources/tests/vote_lib/multi_action.test.move index 9b124a504..204e6e734 100644 --- a/framework/libra-framework/sources/ol_sources/tests/vote_lib/multi_action.test.move +++ b/framework/libra-framework/sources/ol_sources/tests/vote_lib/multi_action.test.move @@ -14,7 +14,7 @@ module ol_framework::test_multi_action { use diem_framework::reconfiguration; // use diem_framework::coin; - // use diem_std::debug::print; + use diem_std::debug::print; struct DummyType has drop, store {} @@ -196,13 +196,13 @@ module ol_framework::test_multi_action { let vals = mock::genesis_n_vals(root, 2); mock::ol_initialize_coin_and_fund_vals(root, 10000000, true); - // make this epoch 1 - // mock::trigger_epoch(root); - + // we are at epoch 0 + let epoch = reconfiguration::get_current_epoch(); + assert!(epoch == 0, 7357001); // Dave creates the resource account. He is not one of the validators, and is not an authority in the multisig. let (resource_sig, _cap) = ol_account::ol_create_resource_account(dave, b"0x1"); let new_resource_address = signer::address_of(&resource_sig); - assert!(resource_account::is_resource_account(new_resource_address), 7357001); + assert!(resource_account::is_resource_account(new_resource_address), 7357002); // fund the account ol_account::transfer(alice, new_resource_address, 100); @@ -219,7 +219,8 @@ module ol_framework::test_multi_action { mock::trigger_epoch(root); // epoch 4 -- now expired let epoch = reconfiguration::get_current_epoch(); - assert!(epoch == 4, 7357004); + print(&epoch); + assert!(epoch == 4, 7357003); // trying to vote on a closed ballot will error let _passed = multi_action::vote_governance(bob, new_resource_address, &id); @@ -325,4 +326,4 @@ module ol_framework::test_multi_action { option::destroy_none(cap_opt); } -} \ No newline at end of file +} diff --git a/framework/libra-framework/sources/ol_sources/tower_state.move b/framework/libra-framework/sources/ol_sources/tower_state.move index 7cbc15f3f..0c2ba8fea 100644 --- a/framework/libra-framework/sources/ol_sources/tower_state.move +++ b/framework/libra-framework/sources/ol_sources/tower_state.move @@ -585,14 +585,20 @@ module ol_framework::tower_state { // pick the next miner // make sure we get an n smaller than list of miners - let k = 0; // k keeps track of this loop, abort if loops too much - while (this_miner_index > count_miners) { - if (k > 1000) return 0; - this_miner_index = this_miner_index / count_miners; - k = k + 1; - }; + // Current case: if miner_index = count_miners could lead to overflow + + // let k = 0; // k keeps track of this loop, abort if loops too much + // while (this_miner_index >= count_miners) { + // if (k > 1000) return 0; + // this_miner_index = this_miner_index / count_miners; + // k = k + 1; + // }; + // double check length of vector - if (this_miner_index >= count_miners ) return 0; + // if (this_miner_index >= count_miners ) return 0; + + // We could use modulo arthemetic on integers + this_miner_index = this_miner_index % count_miners; let miner_addr = vector::borrow
(&all_miners, this_miner_index); let vec = if (exists(*miner_addr)) {