From 76fe4cadb4f1fbfa4f29e533bd33543864e70b0a Mon Sep 17 00:00:00 2001
From: thounyy <thouny@tuta.io>
Date: Fri, 11 Oct 2024 04:44:24 +0200
Subject: [PATCH] chore: remove next_to_destroy

---
 packages/account/sources/account.move    | 13 ++--------
 packages/account/sources/executable.move | 33 +++++++++---------------
 packages/account/sources/proposals.move  | 18 +++++++++++--
 3 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/packages/account/sources/account.move b/packages/account/sources/account.move
index a82b157..961a12e 100644
--- a/packages/account/sources/account.move
+++ b/packages/account/sources/account.move
@@ -297,17 +297,8 @@ public fun outcome_mut<Config, Outcome, W: drop>(
 // === Test functions ===
 
 #[test_only]
-public macro fun deps_mut_for_testing(
-    $account: &mut Account<_, _>, 
+public fun deps_mut_for_testing<Config, Outcome>(
+    account: &mut Account<Config, Outcome>, 
 ): &mut Deps {
-    let account = $account;
     &mut account.deps
-}
-
-#[test_only]
-public macro fun name_mut_for_testing(
-    $account: &mut Account<_, _>, 
-): &mut String {
-    let account = $account;
-    &mut account.name
 }
\ No newline at end of file
diff --git a/packages/account/sources/executable.move b/packages/account/sources/executable.move
index 0917c19..a6ea1e9 100644
--- a/packages/account/sources/executable.move
+++ b/packages/account/sources/executable.move
@@ -10,14 +10,16 @@ module account_protocol::executable;
 use sui::bag::Bag;
 use account_protocol::source::Source;
 
+// === Errors ===
+
+const EActionNotFound: u64 = 0;
+
 // === Structs ===
 
 /// Hot potato ensuring the action in the proposal is executed as it can't be stored
 public struct Executable {
     // module that issued the proposal and must destroy it
     source: Source,
-    // index of the next action to destroy, starts at 0
-    next_to_destroy: u64,
     // actions to be executed in order, heterogenous array
     actions: Bag,
 }
@@ -45,10 +47,8 @@ public fun remove_action<A: store, W: drop>(
 ): A {
     executable.source.assert_is_constructor(witness);
 
-    let next = executable.next_to_destroy;
-    executable.next_to_destroy = next + 1;
-
-    executable.actions.remove(next)
+    let idx = executable.action_index<A>();
+    executable.actions.remove(idx)
 }
 
 /// Completes the execution
@@ -72,25 +72,17 @@ public fun source(executable: &Executable): &Source {
     &executable.source
 }
 
-public fun next_to_destroy(executable: &Executable): u64 {
-    executable.next_to_destroy
-}
-
 public fun actions_length(executable: &Executable): u64 {
     executable.actions.length()
 }
 
 public fun action_index<A: store>(executable: &Executable): u64 {
-    let mut idx = executable.next_to_destroy;
-    let last_idx = idx + executable.actions.length();
-
-    loop {
-        if (
-            idx == last_idx || // returns length if action not found
-            executable.actions.contains_with_type<u64, A>(idx)
-        ) break idx;
-        idx = idx + 1;
-    };
+    let mut idx = 0;
+    executable.actions.length().do!(|i| {
+        if (executable.actions.contains_with_type<u64, A>(i)) idx = i;
+        // returns length if not found
+    });
+    assert!(idx != executable.actions.length(), EActionNotFound);
 
     idx
 }
@@ -106,7 +98,6 @@ public fun action<A: store>(executable: &Executable): &A {
 public(package) fun new(source: Source, actions: Bag): Executable {
     Executable { 
         source,
-        next_to_destroy: 0,
         actions
     }
 }
\ No newline at end of file
diff --git a/packages/account/sources/proposals.move b/packages/account/sources/proposals.move
index f6b732f..3f2f82c 100644
--- a/packages/account/sources/proposals.move
+++ b/packages/account/sources/proposals.move
@@ -20,6 +20,7 @@ const ECantBeExecutedYet: u64 = 0;
 const EHasntExpired: u64 = 1;
 const EProposalNotFound: u64 = 2;
 const EProposalKeyAlreadyExists: u64 = 3;
+const EActionNotFound: u64 = 4;
 
 // === Structs ===
 
@@ -190,8 +191,8 @@ public(package) fun delete<Outcome>(
 
 /// After calling `account::delete_proposal`, delete each action in its own module
 public fun remove_expired_action<Outcome, A: store>(expired: &mut Expired<Outcome>) : A {
-    let action = expired.actions.remove(expired.next_to_destroy);
-    expired.next_to_destroy = expired.next_to_destroy + 1;
+    let idx = action_index<Outcome, A>(expired);
+    let action = expired.actions.remove(idx);
     
     action
 }
@@ -202,4 +203,17 @@ public fun remove_expired_outcome<Outcome>(expired: Expired<Outcome>) : Outcome
     actions.destroy_empty();
 
     outcome
+}
+
+// === Private functions ===
+
+fun action_index<Outcome, A: store>(expired: &Expired<Outcome>): u64 {
+    let mut idx = 0;
+    expired.actions.length().do!(|i| {
+        if (expired.actions.contains_with_type<u64, A>(i)) idx = i;
+        // returns length if not found
+    });
+    assert!(idx != expired.actions.length(), EActionNotFound);
+
+    idx
 }
\ No newline at end of file