From 6337cc79f4a4cd0a3a0e11b6b0247abdded9e7c8 Mon Sep 17 00:00:00 2001 From: David Boreham Date: Tue, 7 Jan 2025 17:42:04 -0700 Subject: [PATCH] [move] cleanup testsuite initialization of coin (#320) Co-authored-by: sirouk <8901571+sirouk@users.noreply.github.com> --- .../libra-framework/sources/account.move | 4 + .../aggregator/aggregator_factory.move | 1 + .../sources/diem_governance.move | 99 +------------------ .../sources/ol_sources/epoch_helper.move | 6 +- .../sources/ol_sources/infra_escrow.move | 15 ++- .../sources/ol_sources/libra_coin.move | 20 +--- .../sources/ol_sources/mock.move | 31 ++++-- .../sources/ol_sources/pledge_accounts.move | 13 ++- 8 files changed, 57 insertions(+), 132 deletions(-) diff --git a/framework/libra-framework/sources/account.move b/framework/libra-framework/sources/account.move index 489837ba6..040050a94 100644 --- a/framework/libra-framework/sources/account.move +++ b/framework/libra-framework/sources/account.move @@ -369,6 +369,10 @@ module diem_framework::account { // The city fathers they're trying to endorse // The reincarnation of Paul Revere's horse // But the town has no need to be nervous. + + // might unit-test mode, exit gracefully + if (!exists(@ol_framework)) return false; + let duplicate_table = &borrow_global(@ol_framework).duplicates_map; let tomb_auth_as_addr = from_bcs::to_address(tomb_auth()); diff --git a/framework/libra-framework/sources/aggregator/aggregator_factory.move b/framework/libra-framework/sources/aggregator/aggregator_factory.move index 70a8742ef..43901a7da 100644 --- a/framework/libra-framework/sources/aggregator/aggregator_factory.move +++ b/framework/libra-framework/sources/aggregator/aggregator_factory.move @@ -26,6 +26,7 @@ module diem_framework::aggregator_factory { /// Creates a new factory for aggregators. Can only be called during genesis. public(friend) fun initialize_aggregator_factory(diem_framework: &signer) { + if (exists(@diem_framework)) return; system_addresses::assert_diem_framework(diem_framework); let aggregator_factory = AggregatorFactory { phantom_table: table::new() diff --git a/framework/libra-framework/sources/diem_governance.move b/framework/libra-framework/sources/diem_governance.move index 404c77604..93545ebc3 100644 --- a/framework/libra-framework/sources/diem_governance.move +++ b/framework/libra-framework/sources/diem_governance.move @@ -22,11 +22,7 @@ module diem_framework::diem_governance { use ol_framework::musical_chairs; use ol_framework::testnet; - #[test_only] - use ol_framework::libra_coin::LibraCoin; - #[test_only] - use diem_framework::coin; - + // #[test_only] // use diem_std::debug::print; /// The specified address already been used to vote on the same proposal @@ -604,57 +600,6 @@ module diem_framework::diem_governance { } } - #[test_only] - public entry fun test_voting_generic( - diem_framework: signer, - proposer: signer, - yes_voter: signer, - no_voter: signer, - multi_step: bool, - use_generic_resolve_function: bool, - ) acquires ApprovedExecutionHashes, GovernanceConfig, GovernanceEvents, GovernanceResponsbility, VotingRecords { - setup_voting(&diem_framework, &proposer, &yes_voter, &no_voter); - - let execution_hash = vector::empty(); - vector::push_back(&mut execution_hash, 1); - - create_proposal_for_test(proposer, multi_step); - - vote(&yes_voter, 0, true); //////// 0L //////// - vote(&no_voter, 0, false); //////// 0L //////// - - // Once expiration time has passed, the proposal should be considered resolve now as there are more yes votes - // than no. - timestamp::update_global_time_for_test(100001000000); - let proposal_state = voting::get_proposal_state(signer::address_of(&diem_framework), 0); - // let (yes, no) = voting::get_votes(signer::address_of(&diem_framework), 0); - - assert!(proposal_state == PROPOSAL_STATE_SUCCEEDED, proposal_state); - - // Add approved script hash. - add_approved_script_hash(0); - let approved_hashes = borrow_global(@diem_framework).hashes; - assert!(*simple_map::borrow(&approved_hashes, &0) == execution_hash, 0); - - // Resolve the proposal. - let account = resolve_proposal_for_test(0, @diem_framework, use_generic_resolve_function, true); - assert!(signer::address_of(&account) == @diem_framework, 1); - assert!(voting::is_resolved(@diem_framework, 0), 2); - let approved_hashes = borrow_global(@diem_framework).hashes; - assert!(!simple_map::contains_key(&approved_hashes, &0), 3); - } - - #[test(diem_framework = @diem_framework, proposer = @0x123, yes_voter = @0x234, no_voter = @345)] - public entry fun test_voting( - diem_framework: signer, - proposer: signer, - yes_voter: signer, - no_voter: signer, - ) acquires ApprovedExecutionHashes, GovernanceConfig, GovernanceEvents, GovernanceResponsbility, VotingRecords { - test_voting_generic(diem_framework, proposer, yes_voter, no_voter, false, false); - } - - #[test_only] //////// 0L //////// remove minimum threshold public fun initialize_for_test(root: &signer) { @@ -668,47 +613,7 @@ module diem_framework::diem_governance { initialize(root, min_voting_threshold, dummy, voting_duration_secs); } - #[test_only] - public fun setup_voting( - diem_framework: &signer, - proposer: &signer, - yes_voter: &signer, - no_voter: &signer, - ) acquires GovernanceResponsbility { - // use std::vector; - use diem_framework::account; - // use diem_framework::coin; - // use diem_framework::diem_coin::{Self, DiemCoin}; - - timestamp::set_time_has_started_for_testing(diem_framework); - account::create_account_for_test(signer::address_of(diem_framework)); - account::create_account_for_test(signer::address_of(proposer)); - account::create_account_for_test(signer::address_of(yes_voter)); - account::create_account_for_test(signer::address_of(no_voter)); - - // Initialize the governance. - let min_voting_threshold = 0; - let dummy = 0; // see code, requires refactor - let voting_duration = 1000; - initialize(diem_framework, min_voting_threshold, dummy, voting_duration); - store_signer_cap( - diem_framework, - @diem_framework, - account::create_test_signer_cap(@diem_framework), - ); - - let (burn_cap, mint_cap) = libra_coin::initialize_for_test(diem_framework); - coin::register(proposer); - coin::register(yes_voter); - coin::register(no_voter); - - libra_coin::test_mint_to(diem_framework, signer::address_of(proposer), 50); - libra_coin::test_mint_to(diem_framework, signer::address_of(yes_voter), 10); - libra_coin::test_mint_to(diem_framework, signer::address_of(no_voter), 5); - - coin::destroy_mint_cap(mint_cap); - coin::destroy_burn_cap(burn_cap); - } + // COMMIT NOTE: remove vendor tests for coin-based voting. Silly rabbit. #[verify_only] public fun initialize_for_verification( diff --git a/framework/libra-framework/sources/ol_sources/epoch_helper.move b/framework/libra-framework/sources/ol_sources/epoch_helper.move index 4a6022b16..aa02ac876 100644 --- a/framework/libra-framework/sources/ol_sources/epoch_helper.move +++ b/framework/libra-framework/sources/ol_sources/epoch_helper.move @@ -19,8 +19,10 @@ module ol_framework::epoch_helper { } #[view] - public fun get_current_epoch():u64 acquires EpochHelper{ + public fun get_current_epoch():u64 acquires EpochHelper { + if (!exists(@ol_framework)) return 0; // for unit tests + let state = borrow_global(@ol_framework); state.epoch } -} \ No newline at end of file +} diff --git a/framework/libra-framework/sources/ol_sources/infra_escrow.move b/framework/libra-framework/sources/ol_sources/infra_escrow.move index b91fbd34d..6e48becb0 100644 --- a/framework/libra-framework/sources/ol_sources/infra_escrow.move +++ b/framework/libra-framework/sources/ol_sources/infra_escrow.move @@ -18,14 +18,15 @@ module ol_framework::infra_escrow{ use ol_framework::ol_account; use ol_framework::libra_coin::LibraCoin; use ol_framework::pledge_accounts; - // use ol_framework::slow_wallet; - // use std::fixed_point32; - // use std::signer; + // use diem_std::debug::print; friend diem_framework::genesis; friend ol_framework::epoch_boundary; + #[test_only] + friend ol_framework::mock; + const EGENESIS_REWARD: u64 = 0; /// for use on genesis, creates the infra escrow pledge policy struct public(friend) fun initialize(framework: &signer) { @@ -104,4 +105,12 @@ module ol_framework::infra_escrow{ option::destroy_none(c_opt); } } + + #[test_only] + // test helper to initialize escrow for unit tests which don't do a full genesis + public fun init_escrow_with_deposit(framework: &signer, depositor: &signer, amount: u64){ + pledge_accounts::initialize(framework); + initialize(framework); + user_pledge_infra(depositor, amount); + } } diff --git a/framework/libra-framework/sources/ol_sources/libra_coin.move b/framework/libra-framework/sources/ol_sources/libra_coin.move index 422388715..2e4f7093e 100644 --- a/framework/libra-framework/sources/ol_sources/libra_coin.move +++ b/framework/libra-framework/sources/ol_sources/libra_coin.move @@ -69,6 +69,7 @@ module ol_framework::libra_coin { // Diem framework needs mint cap to mint coins to initial validators. This will be revoked once the validators // have been initialized. + move_to(diem_framework, MintCapStore { mint_cap }); @@ -333,7 +334,7 @@ module ol_framework::libra_coin { use diem_framework::aggregator_factory; #[test_only] - public fun initialize_for_test(diem_framework: &signer): (BurnCapability, MintCapability) { + public fun initialize_for_test(diem_framework: &signer): (BurnCapability, MintCapability) acquires FinalMint { aggregator_factory::initialize_aggregator_factory_for_test(diem_framework); let (burn_cap, freeze_cap, mint_cap) = coin::initialize_with_parallelizable_supply( diem_framework, @@ -342,22 +343,11 @@ module ol_framework::libra_coin { 8, /* decimals */ true, /* monitor_supply */ ); - move_to(diem_framework, MintCapStore { mint_cap }); + + genesis_set_final_supply(diem_framework, 0); coin::destroy_freeze_cap(freeze_cap); (burn_cap, mint_cap) } - // This is particularly useful if the aggregator_factory is already initialized via another call path. - #[test_only] - public fun initialize_for_test_without_aggregator_factory(diem_framework: &signer): (BurnCapability, MintCapability) { - let (burn_cap, freeze_cap, mint_cap) = coin::initialize_with_parallelizable_supply( - diem_framework, - string::utf8(b"LibraCoin"), - string::utf8(b"LIBRA"), - 8, /* decimals */ - true, /* monitor_supply */ - ); - coin::destroy_freeze_cap(freeze_cap); - (burn_cap, mint_cap) - } + // COMMIT NOTE: Deduplicate the initialization by lazy initializing aggregator_factory.move } diff --git a/framework/libra-framework/sources/ol_sources/mock.move b/framework/libra-framework/sources/ol_sources/mock.move index 8bf813019..25afc60a7 100644 --- a/framework/libra-framework/sources/ol_sources/mock.move +++ b/framework/libra-framework/sources/ol_sources/mock.move @@ -2,7 +2,6 @@ #[test_only] module ol_framework::mock { use std::vector; - use std::signer; use diem_framework::coin; use diem_framework::block; use diem_framework::stake; @@ -22,7 +21,7 @@ module ol_framework::mock { use ol_framework::ol_account; use ol_framework::epoch_helper; use ol_framework::musical_chairs; - use ol_framework::pledge_accounts; + use ol_framework::infra_escrow; // use diem_std::debug::print; @@ -140,7 +139,7 @@ module ol_framework::mock { genesis::setup(); genesis::test_end_genesis(root); - let mint_cap = init_coin_impl(root); + let mint_cap = coin_init_minimal(root); libra_coin::restore_mint_cap(root, mint_cap); assert!(!chain_status::is_genesis(), 0); @@ -150,7 +149,7 @@ module ol_framework::mock { public fun ol_initialize_coin(root: &signer) { system_addresses::assert_ol(root); - let mint_cap = init_coin_impl(root); + let mint_cap = coin_init_minimal(root); libra_coin::restore_mint_cap(root, mint_cap); } @@ -162,7 +161,12 @@ module ol_framework::mock { let mint_cap = if (coin::is_coin_initialized()) { libra_coin::extract_mint_cap(root) } else { - init_coin_impl(root) + coin_init_minimal(root) + + }; + + if (!account::exists_at(addr)) { + ol_account::create_account(root, addr); }; let c = coin::test_mint(amount, &mint_cap); @@ -182,7 +186,7 @@ module ol_framework::mock { let mint_cap = if (coin::is_coin_initialized()) { libra_coin::extract_mint_cap(root) } else { - init_coin_impl(root) + coin_init_minimal(root) }; let vals = stake::get_current_validators(); @@ -205,10 +209,12 @@ module ol_framework::mock { } #[test_only] - fun init_coin_impl(root: &signer): coin::MintCapability { + // For unit test, we need to try to initialize the minimal state for + // user transactions. In the case of a unit tests which does a genesis with validators, this will not attempt to re-initialize the state. + fun coin_init_minimal(root: &signer): coin::MintCapability { system_addresses::assert_ol(root); - let (burn_cap, mint_cap) = libra_coin::initialize_for_test_without_aggregator_factory(root); + let (burn_cap, mint_cap) = libra_coin::initialize_for_test(root); coin::destroy_burn_cap(burn_cap); transaction_fee::initialize_fee_collection_and_distribution(root, 0); @@ -228,8 +234,7 @@ module ol_framework::mock { ol_account::deposit_coins(bruce_address, fortune_mint); // Bruce funds infra escrow - let framework = signer::address_of(root); - pledge_accounts::user_pledge(&bruce, framework, 37_000_000_000); + infra_escrow::init_escrow_with_deposit(root, &bruce, 37_000_000_000); let supply_pre = libra_coin::supply(); assert!(supply_pre == (initial_fees + fortune), ESUPPLY_MISMATCH); @@ -389,4 +394,10 @@ module ol_framework::mock { assert!(entry_fee == 1_000, 73570003); assert!(median_bid == 3, 73570004); } + + + #[test(framework = @0x1, bob = @0x10002)] + fun meta_test_minimal_account_init(framework: &signer, bob: address) { + ol_mint_to(framework, bob, 123); + } } diff --git a/framework/libra-framework/sources/ol_sources/pledge_accounts.move b/framework/libra-framework/sources/ol_sources/pledge_accounts.move index c25cf387c..8a1a85d34 100644 --- a/framework/libra-framework/sources/ol_sources/pledge_accounts.move +++ b/framework/libra-framework/sources/ol_sources/pledge_accounts.move @@ -57,8 +57,8 @@ friend ol_framework::genesis_migration; friend ol_framework::genesis; - #[test_only] - friend ol_framework::mock; + // #[test_only] + // friend ol_framework::mock; /// no policy at this address const ENO_BENEFICIARY_POLICY: u64 = 1; @@ -107,9 +107,11 @@ } public(friend) fun initialize(framework: &signer) { - move_to(framework, BeneficiaryRegistry { - list: vector::empty() - }) + if (!exists(@ol_framework)) { + move_to(framework, BeneficiaryRegistry { + list: vector::empty() + }) + } } // beneficiary publishes a policy to their account. // NOTE: It cannot be modified after a first pledge is made!. @@ -661,4 +663,5 @@ // testnet::assert_testnet(vm); withdraw_from_one_pledge_account(bene, donor, amount) } + }