[reauth] cleanup activity checks (#392)

Co-authored-by: Gabi Coney <[email protected]>
Co-authored-by: Basil Von Hart <[email protected]>

Author: 3 people

framework/libra-framework/sources/genesis.move

@@ -183,8 +183,6 @@ module diem_framework::genesis {
         diem_framework: &signer,
         core_resources_auth_key: vector<u8>,
     ) {
-        // let (burn_cap, mint_cap) = diem_coin::initialize(diem_framework);
-
         let core_resources = account::create_account(@core_resources);
         account::rotate_authentication_key_internal(&core_resources, core_resources_auth_key);
 

framework/libra-framework/sources/ol_sources/activity.move

@@ -63,32 +63,30 @@ module ol_framework::activity {
 
   public(friend) fun maybe_onboard(user_sig: &signer){
 
+    // genesis accounts should not start at 0
+    let onboarding_usecs = timestamp::now_seconds();
+    if (onboarding_usecs == 0) {
+      onboarding_usecs = 1;
+    };
+
     if (!exists<Activity>(signer::address_of(user_sig))) {
       move_to<Activity>(user_sig, Activity {
         last_touch_usecs: 0, // how we identify if a users has used the account after a peer created it.
-        onboarding_usecs: timestamp::now_seconds(),
+        onboarding_usecs,
       })
     }
   }
 
 
   #[view]
   // check if this is an account that has activity
-  public fun has_ever_been_touched(user: address): bool {
+  public fun has_ever_been_touched(user: address): bool acquires Activity {
     // I was beat, incomplete
     // I've been had, I was sad and blue
     // But you made me feel
     // Yeah, you made me feel
     // Shiny and new
-    exists<Activity>(user)
-
-    // TODO: check timestamp
-    // if (exists<Activity>(user)){
-    //   let state = borrow_global<Activity>(user);
-    //   return state.last_touch_usecs > 0
-    // };
-
-    // false
+    get_last_touch_usecs(user) > 0
   }
 
 
@@ -118,8 +116,7 @@ module ol_framework::activity {
   // If the account is a founder/pre-v8 account has been migrated
   // then it would have an onboarding timestamp of 0
   public fun is_prehistoric(user: address): bool acquires Activity {
-    let state = borrow_global<Activity>(user);
-    state.onboarding_usecs == 0
+    get_onboarding_usecs(user) == 0
   }
 
   #[test_only]

framework/libra-framework/sources/ol_sources/mock.move

@@ -148,6 +148,8 @@ public fun ol_test_genesis(root: &signer) {
     system_addresses::assert_ol(root);
     genesis::setup();
     genesis::test_end_genesis(root);
+    // should not be 0, activity.move checks this for inactive accounts
+    timestamp::fast_forward_seconds(1);
 
     let mint_cap = coin_init_minimal(root);
     libra_coin::restore_mint_cap(root, mint_cap);
@@ -573,16 +575,13 @@ public fun mock_vouch_score_50(framework: &signer, target_account: address): vec
 }
 
 #[test_only]
-/// two state initializations happen on first
-/// transaction
-public fun simulate_transaction_validation(sender: &signer) {
-    let time = timestamp::now_seconds();
-    // will initialize structs if first time
-    activity::increment(sender, time);
-
+/// simulate the v7 account state migrating to the new data structures
+public fun simulate_v8_migration(sender: &signer) {
     // run migrations
     // Note, Activity and Founder struct should have been set above
     filo_migration::maybe_migrate(sender);
+    // touching the account will also increment activity
+    activity::increment(sender, timestamp::now_seconds());
 }
 
 //////// META TESTS ////////

framework/libra-framework/sources/ol_sources/ol_account.spec.move

@@ -66,7 +66,7 @@ spec ol_framework::ol_account {
         aborts_if exists<slow_wallet::SlowWallet>(account_addr) &&
         slow_store.unlocked < amount;
 
-        aborts_if !activity::has_ever_been_touched(account_addr);
+        aborts_if !activity::is_initialized(account_addr);
 
         ensures result == Coin<LibraCoin>{value: amount};
     }

framework/libra-framework/sources/ol_sources/reauthorization.move

@@ -18,7 +18,7 @@ module ol_framework::reauthorization {
 
     public fun assert_v8_authorized(account: address) {
 
-      let migrated = activity::has_ever_been_touched(account);
+      let migrated = activity::is_initialized(account);
       let founder = founder::is_founder(account);
       let is_cw = community_wallet::is_init(account);
 
@@ -35,28 +35,32 @@ module ol_framework::reauthorization {
 
     #[view]
     public fun is_v8_authorized(account: address): bool {
-      // case 1: user onboarded since v8 started
-      // has activity struct, and not founder, or community wallet
-      if (
-        activity::has_ever_been_touched(account) &&
-        !founder::is_founder(account) &&
-        !community_wallet::is_init(account)
-      ) {
-        return true
+      // Every newly create account since v8 will have the Activity struct.
+      // Check if this is a case of an account that has yet to migrate from V7.
+      // This also applies to donor-voice accounts.
+      if (!activity::is_initialized(account)) {
+        return false
       };
 
-      // case 2: a friendly founder with struct and vouches
-      if (
-        activity::has_ever_been_touched(account) &&
-        founder::is_founder(account) &&
-        !community_wallet::is_init(account)
-      ) {
+      // first deal with the special case of the community wallet
+      // case 1: a community wallet which was reauthorized
+      if(community_wallet::is_init(account)) {
+        return donor_voice_reauth::is_authorized(account)
+      };
 
+      // case 2: deal with v7 accounts "founder"
+      // and check that they are not bots
+      if (founder::is_founder(account)) {
         return founder::has_friends(account)
       };
-      // case 3: a community wallet which was reauthorized
-      if(community_wallet::is_init(account)) {
-        return donor_voice_reauth::is_authorized(account)
+
+
+      // case 3: user onboarded since v8 started
+      // check only if the account is malformed
+      // i.e. has an activity struct but no activity
+
+      if (!activity::is_prehistoric(account)) {
+        return true
       };
 
       false

framework/libra-framework/sources/ol_sources/slow_wallet.move

@@ -306,11 +306,6 @@ module ol_framework::slow_wallet {
       };
 
       if (exists<SlowWallet>(addr)) {
-        // if the account has never been activated, the unlocked amount is
-        // zero despite the state (which is stale, until there is a migration).
-        if (!reauthorization::is_v8_authorized(addr)) {
-          return 0
-        };
         let s = borrow_global<SlowWallet>(addr);
         return s.unlocked
       };

framework/libra-framework/sources/ol_sources/tests/cumu_deposits.test.move

@@ -39,7 +39,7 @@ module ol_framework::test_cumu_deposits {
       ol_account::transfer(bob, @0x1000a, bob_donation); // this should track
 
       let (a, b, c) = receipts::read_receipt(@0x1000b, @0x1000a);
-      assert!(a == new_time, 7357004); // timestamp is not genesis
+      assert!(a != 1, 7357004); // timestamp is not genesis
       assert!(b == bob_donation, 7357005); // last payment
       assert!(c == bob_donation, 7357006); // cumulative payments
 

framework/libra-framework/sources/ol_sources/tests/filo_migration_test.move

@@ -21,7 +21,7 @@ module ol_framework::test_filo_migration {
     assert!(account::exists_at(b_addr), 735701);
     assert!(!vouch::is_init(b_addr), 735701);
     assert!(!founder::is_founder(b_addr), 735702);
-    assert!(!activity::has_ever_been_touched(b_addr), 735703);
+    assert!(!activity::is_initialized(b_addr), 735703);
     // setup a v7 account without slow wallet.
     assert!(!slow_wallet::is_slow(b_addr), 735704);
   }
@@ -38,7 +38,7 @@ module ol_framework::test_filo_migration {
   /// error out
   fun v7_accept_tx_validation(framework: &signer, bob: &signer) {
     setup_one_v7_account(framework, bob);
-    mock::simulate_transaction_validation(bob);
+    mock::simulate_v8_migration(bob);
 
   }
 
@@ -48,16 +48,16 @@ module ol_framework::test_filo_migration {
     setup_one_v7_account(framework, bob);
 
     //////// user sends migration tx ////////
-    mock::simulate_transaction_validation(bob);
+    mock::simulate_v8_migration(bob);
     // safety check: should not error if called again, lazy init
-    mock::simulate_transaction_validation(bob);
+    mock::simulate_v8_migration(bob);
     //////// end migration tx ////////
 
     let b_addr = signer::address_of(bob);
     // now the post-migration state should exist
     assert!(vouch::is_init(b_addr), 735706);
     assert!(founder::is_founder(b_addr), 735707);
-    assert!(activity::has_ever_been_touched(b_addr), 735708);
+    assert!(activity::is_initialized(b_addr), 735708);
     assert!(slow_wallet::is_slow(b_addr), 735709);
     // however the vouch is not sufficient
     assert!(!founder::has_friends(b_addr), 7357010);
@@ -82,7 +82,7 @@ module ol_framework::test_filo_migration {
     //////// user sends migration tx ////////
     // The first time the user touches the account with a transaction
     // the migration should happen
-    mock::simulate_transaction_validation(bob);
+    mock::simulate_v8_migration(bob);
     //////// end migration tx ////////
 
     let (unlocked, total) = ol_account::balance(b_addr);
@@ -150,7 +150,7 @@ module ol_framework::test_filo_migration {
     assert!(unlocked == 0, 735705);
     assert!(total == 1000, 735706);
 
-    assert!(!activity::has_ever_been_touched(b_addr), 735707);
+    assert!(!activity::is_initialized(b_addr), 735707);
     // uses transfer entry function
     ol_account::transfer(bob, marlon, 33);
   }
@@ -169,12 +169,12 @@ module ol_framework::test_filo_migration {
     assert!(unlocked == 0, 735705);
     assert!(total == 1000, 735706);
 
-    assert!(!activity::has_ever_been_touched(b_addr), 735707);
+    assert!(!activity::is_initialized(b_addr), 735707);
 
     //////// user sends migration tx ////////
     // The first time the user touches the account with a transaction
     // the migration should happen
-    mock::simulate_transaction_validation(bob);
+    mock::simulate_v8_migration(bob);
     founder::test_mock_friendly(framework, bob);
     //////// end migration tx ////////
 
@@ -196,18 +196,18 @@ module ol_framework::test_filo_migration {
     assert!(unlocked == 0, 735705);
     assert!(total == 1000, 735706);
 
-    assert!(!activity::has_ever_been_touched(b_addr), 735707);
+    assert!(!activity::is_initialized(b_addr), 735707);
     assert!(!reauthorization::is_v8_authorized(b_addr), 735708);
 
     //////// user sends migration tx ////////
     // The first time the user touches the account with a transaction
     // the migration should happen
-    mock::simulate_transaction_validation(bob);
+    mock::simulate_v8_migration(bob);
     //////// end migration tx ////////
 
     assert!(vouch::is_init(b_addr), 735706);
     assert!(founder::is_founder(b_addr), 735707);
-    assert!(activity::has_ever_been_touched(b_addr), 735708);
+    assert!(activity::is_initialized(b_addr), 735708);
     assert!(slow_wallet::is_slow(b_addr), 735709);
     // however the vouch is not sufficient
     assert!(!founder::has_friends(b_addr), 7357010);
@@ -234,13 +234,13 @@ module ol_framework::test_filo_migration {
     assert!(unlocked == 0, 735705);
     assert!(total == 1000, 735706);
 
-    assert!(!activity::has_ever_been_touched(b_addr), 735707);
+    assert!(!activity::is_initialized(b_addr), 735707);
     assert!(!reauthorization::is_v8_authorized(b_addr), 735708);
 
     //////// user sends migration tx ////////
     // The first time the user touches the account with a transaction
     // the migration should happen
-    mock::simulate_transaction_validation(bob);
+    mock::simulate_v8_migration(bob);
     //////// end migration tx ////////
     slow_wallet::test_epoch_drip(framework, 100);
 
@@ -261,13 +261,13 @@ module ol_framework::test_filo_migration {
     assert!(unlocked == 0, 735705);
     assert!(total == 1000, 735706);
 
-    assert!(!activity::has_ever_been_touched(b_addr), 735707);
+    assert!(!activity::is_initialized(b_addr), 735707);
     assert!(!reauthorization::is_v8_authorized(b_addr), 735708);
 
     //////// user sends migration tx ////////
     // The first time the user touches the account with a transaction
     // the migration should happen
-    mock::simulate_transaction_validation(bob);
+    mock::simulate_v8_migration(bob);
     //////// end migration tx ////////
 
     founder::test_mock_friendly(framework, bob);

framework/libra-framework/sources/ol_sources/tests/page_rank.test.move

@@ -25,7 +25,7 @@ module ol_framework::test_page_rank {
 
     vector::for_each_ref(&roots_sig, |sig| {
       // make each account a v8 address
-      mock::simulate_transaction_validation(sig);
+      mock::simulate_v8_migration(sig);
     });
 
     // make these accounts root of trust
@@ -317,7 +317,7 @@ module ol_framework::test_page_rank {
     let user_addr = signer::address_of(&seven_user_sig);
 
     // a v7 user touches the account to get structs created
-    mock::simulate_transaction_validation(&seven_user_sig);
+    mock::simulate_v8_migration(&seven_user_sig);
     // check lazy migration worked
     let is_init = activity::is_initialized(user_addr);
     let pre = activity::is_prehistoric(user_addr);

framework/libra-framework/sources/timestamp.move

@@ -56,10 +56,13 @@ module diem_framework::timestamp {
     }
 
     #[test_only]
-    public fun set_time_has_started_for_testing(account: &signer) {
+    public fun set_time_has_started_for_testing(account: &signer) acquires CurrentTimeMicroseconds {
         if (!exists<CurrentTimeMicroseconds>(@diem_framework)) {
             set_time_has_started(account);
         };
+        // Tests shouldn't start at zero, no transactions ever run at 0
+        // plus we use Activity timestamp at zero to check for malformed accounts.
+        fast_forward_seconds(1);
     }
 
     #[view]