Address PR comments

radixdlt · Aug 30, 2024 · 3db2d64 · 3db2d64
1 parent ae56381
commit 3db2d64
Show file tree

Hide file tree

Showing 14 changed files with 115 additions and 24 deletions.
diff --git a/...gon/Extensions/SampleValues/Transaction/AccountLockerClaimableResource+SampleValues.swift b/...gon/Extensions/SampleValues/Transaction/AccountLockerClaimableResource+SampleValues.swift
@@ -0,0 +1,8 @@
+import SargonUniFFI
+
+#if DEBUG
+extension AccountLockerClaimableResource {
+	public static let sample: Self = newAccountLockerClaimableResourceSample()
+	public static let sampleOther: Self = newAccountLockerClaimableResourceSampleOther()
+}
+#endif // DEBUG
diff --git a/apple/Sources/Sargon/Extensions/Swiftified/RET/TransactionManifest+Swiftified.swift b/apple/Sources/Sargon/Extensions/Swiftified/RET/TransactionManifest+Swiftified.swift
@@ -63,12 +63,14 @@ extension TransactionManifest {
     public static func accountLockerClaim(
         lockerAddress: LockerAddress,
         claimant: AccountAddress,
-        claimableResources: [AccountLockerClaimableResource]
+        claimableResources: [AccountLockerClaimableResource],
+        useTryDepositOrAbort: Bool
     ) -> Self {
         manifestAccountLockerClaim(
             lockerAddress: lockerAddress,
             claimant: claimant,
-            claimableResources: claimableResources
+            claimableResources: claimableResources,
+            useTryDepositOrAbort: useTryDepositOrAbort
         )
     }
 }

diff --git a/.../Sargon/Extensions/Swiftified/Transaction/AccountLockerClaimableResource+Swiftified.swift b/.../Sargon/Extensions/Swiftified/Transaction/AccountLockerClaimableResource+Swiftified.swift
@@ -0,0 +1,4 @@
+import Foundation
+import SargonUniFFI
+
+extension AccountLockerClaimableResource: SargonModel {}
diff --git a/apple/Tests/TestCases/RET/ManifestBuilding/ManifestBuildingTests.swift b/apple/Tests/TestCases/RET/ManifestBuilding/ManifestBuildingTests.swift
@@ -221,7 +221,8 @@ final class ManifestBuildingTests: Test<TransactionManifest> {
             let manifest = SUT.accountLockerClaim(
 				lockerAddress: LockerAddress.sample,
 				claimant: accountAddress,
-				claimableResources: [AccountLockerClaimableResource.fungible(resourceAddress: ResourceAddress.sample, amount: Decimal192.sample)]
+				claimableResources: [AccountLockerClaimableResource.fungible(resourceAddress: ResourceAddress.sample, amount: Decimal192.sample)],
+				useTryDepositOrAbort: false
 			)
             XCTAssert(manifest.description.contains(accountAddress.address))
             XCTAssertEqual(manifest.description.ranges(of: ";").count, 2) // 2 instructions

diff --git a/...argon/src/wrapped_radix_engine_toolkit/high_level/manifest_building/account_locker/mod.rs b/...argon/src/wrapped_radix_engine_toolkit/high_level/manifest_building/account_locker/mod.rs
diff --git a/...account_locker/manifest_account_locker.rs → ...ifest_building/manifest_account_locker.rs b/...account_locker/manifest_account_locker.rs → ...ifest_building/manifest_account_locker.rs
@@ -1,4 +1,5 @@
 use crate::prelude::*;
+use radix_common::prelude::ManifestExpression;
 use radix_engine_interface::blueprints::locker::{
     ACCOUNT_LOCKER_CLAIM_IDENT, ACCOUNT_LOCKER_CLAIM_NON_FUNGIBLES_IDENT,
 };
@@ -14,14 +15,15 @@ impl TransactionManifest {
         locker_address: &LockerAddress,
         claimant: &AccountAddress,
         claimable_resources: Vec<AccountLockerClaimableResource>,
+        use_try_deposit_or_abort: bool,
     ) -> Self {
         let mut builder = ScryptoManifestBuilder::new();
         let claimant_arg: ScryptoComponentAddress = (*claimant).into();
         let claimable_resources =
             Self::build_claimable_batch(claimable_resources, 50);
 
-        for claimable in claimable_resources.clone() {
-            match claimable {
+        for claimable in claimable_resources.iter() {
+            match claimable.clone() {
                 AccountLockerClaimableResource::Fungible {
                     resource_address,
                     amount,
@@ -55,7 +57,15 @@ impl TransactionManifest {
         }
 
         if !claimable_resources.is_empty() {
-            builder = builder.deposit_batch(claimant_arg);
+            builder = if use_try_deposit_or_abort {
+                builder.try_deposit_batch_or_abort(
+                    claimant_arg,
+                    ManifestExpression::EntireWorktop,
+                    None,
+                )
+            } else {
+                builder.deposit_batch(claimant_arg)
+            }
         }
         TransactionManifest::sargon_built(builder, claimant.network_id())
     }
@@ -68,12 +78,7 @@ impl TransactionManifest {
         claimable_resources
             .into_iter()
             .take_while(|claimable| {
-                current_batch_size += match claimable {
-                    AccountLockerClaimableResource::Fungible { .. } => 1,
-                    AccountLockerClaimableResource::NonFungible {
-                        ids, ..
-                    } => ids.len(),
-                };
+                current_batch_size += claimable.resource_count();
                 current_batch_size < size
             })
             .collect()
@@ -93,7 +98,8 @@ mod tests {
         let manifest = SUT::account_locker_claim(
             &"locker_rdx1drn4q2zk6dvljehytnhfah330xk7emfznv59rqlps5ayy52d7xkzzz".into(),
             &"account_rdx128y6j78mt0aqv6372evz28hrxp8mn06ccddkr7xppc88hyvynvjdwr".into(),
-            vec![]
+            vec![],
+            false
         );
         manifest_eq(manifest, "")
     }
@@ -130,6 +136,7 @@ mod tests {
                     ids: vec![NonFungibleLocalId::string("foobar").unwrap()],
                 },
             ],
+            false
         );
 
         manifest_eq(manifest, expected_manifest)
@@ -144,6 +151,7 @@ mod tests {
                 resource_address: "resource_rdx1n2ekdd2m0jsxjt9wasmu3p49twy2yfalpaa6wf08md46sk8dfmldnd".into(),
                 ids: vec![NonFungibleLocalId::integer(i)]
             }).collect(),
+            true
         );
 
         assert_eq!(manifest.instructions().len(), 50)

diff --git a/crates/sargon/src/wrapped_radix_engine_toolkit/high_level/manifest_building/mod.rs b/crates/sargon/src/wrapped_radix_engine_toolkit/high_level/manifest_building/mod.rs
@@ -1,20 +1,20 @@
-mod account_locker;
 mod addresses_manifest_builder_support;
 mod assert_manifest;
 mod bucket;
 mod bucket_factory;
+mod manifest_account_locker;
 mod manifest_assets_transfers;
 mod manifests;
 mod manifests_create_tokens;
 mod metadata;
 mod modify_manifest;
 mod third_party_deposit_update;
 
-pub use account_locker::*;
 pub use addresses_manifest_builder_support::*;
 pub use assert_manifest::*;
 pub use bucket::*;
 pub use bucket_factory::*;
+pub use manifest_account_locker::*;
 pub use manifest_assets_transfers::*;
 pub use manifests::*;
 pub use manifests_create_tokens::*;

diff --git a/crates/sargon/src/wrapped_radix_engine_toolkit/high_level/ret_api.rs b/crates/sargon/src/wrapped_radix_engine_toolkit/high_level/ret_api.rs
@@ -173,11 +173,13 @@ pub fn manifest_account_locker_claim(
     locker_address: &LockerAddress,
     claimant: &AccountAddress,
     claimable_resources: Vec<AccountLockerClaimableResource>,
+    use_try_deposit_or_abort: bool,
 ) -> TransactionManifest {
     TransactionManifest::account_locker_claim(
         locker_address,
         claimant,
         claimable_resources,
+        use_try_deposit_or_abort,
     )
 }
 
@@ -482,6 +484,7 @@ mod tests {
             &LockerAddress::sample(),
             &AccountAddress::sample(),
             vec![AccountLockerClaimableResource::sample()],
+            true,
         );
         assert_eq!(manifest.instructions().len(), 2);
     }

diff --git a/...cker/account_locker_claimable_resource.rs → ...cker/account_locker_claimable_resource.rs b/...cker/account_locker_claimable_resource.rs → ...cker/account_locker_claimable_resource.rs
@@ -1,17 +1,29 @@
 use crate::prelude::*;
 
+/// A claimable resource in an account locker.
 #[derive(Debug, Clone, PartialEq, Eq, Hash, uniffi::Enum)]
 pub enum AccountLockerClaimableResource {
+    /// A fungible resource with a specific claimable amount
     Fungible {
         resource_address: ResourceAddress,
         amount: Decimal192,
     },
+    /// A non-fungible resource with specific claimable IDs
     NonFungible {
         resource_address: ResourceAddress,
         ids: Vec<NonFungibleLocalId>,
     },
 }
 
+impl AccountLockerClaimableResource {
+    pub fn resource_count(&self) -> usize {
+        match self {
+            Self::Fungible { .. } => 1,
+            Self::NonFungible { ids, .. } => ids.len(),
+        }
+    }
+}
+
 impl HasSampleValues for AccountLockerClaimableResource {
     fn sample() -> Self {
         Self::Fungible {

diff --git a/...level/sargon_specific_types/account_locker/account_locker_claimable_resource_uniffi_fn.rs b/...level/sargon_specific_types/account_locker/account_locker_claimable_resource_uniffi_fn.rs
@@ -0,0 +1,36 @@
+use crate::prelude::*;
+
+#[uniffi::export]
+pub fn new_account_locker_claimable_resource_sample(
+) -> AccountLockerClaimableResource {
+    AccountLockerClaimableResource::sample()
+}
+
+#[uniffi::export]
+pub fn new_account_locker_claimable_resource_sample_other(
+) -> AccountLockerClaimableResource {
+    AccountLockerClaimableResource::sample_other()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[allow(clippy::upper_case_acronyms)]
+    type SUT = AccountLockerClaimableResource;
+
+    #[test]
+    fn hash_of_samples() {
+        assert_eq!(
+            HashSet::<SUT>::from_iter([
+                new_account_locker_claimable_resource_sample(),
+                new_account_locker_claimable_resource_sample_other(),
+                // duplicates should get removed
+                new_account_locker_claimable_resource_sample(),
+                new_account_locker_claimable_resource_sample_other(),
+            ])
+            .len(),
+            2
+        );
+    }
+}
diff --git a/...n/src/wrapped_radix_engine_toolkit/high_level/sargon_specific_types/account_locker/mod.rs b/...n/src/wrapped_radix_engine_toolkit/high_level/sargon_specific_types/account_locker/mod.rs
@@ -0,0 +1,5 @@
+mod account_locker_claimable_resource;
+mod account_locker_claimable_resource_uniffi_fn;
+
+pub use account_locker_claimable_resource::*;
+pub use account_locker_claimable_resource_uniffi_fn::*;
diff --git a/crates/sargon/src/wrapped_radix_engine_toolkit/high_level/sargon_specific_types/mod.rs b/crates/sargon/src/wrapped_radix_engine_toolkit/high_level/sargon_specific_types/mod.rs
@@ -1,6 +1,7 @@
 #[macro_use]
 mod address_union;
 
+mod account_locker;
 mod address_of_account_or_persona;
 mod assets_transfers;
 mod build_information;
@@ -17,3 +18,5 @@ pub use address_union::*;
 pub use stake_claim::*;
 pub use stake_claim_uniffi_fn::*;
 pub use transaction_guarantee::*;
+
+pub use account_locker::*;
diff --git a/jvm/sargon-android/src/main/java/com/radixdlt/sargon/extensions/TransactionManifest.kt b/jvm/sargon-android/src/main/java/com/radixdlt/sargon/extensions/TransactionManifest.kt
@@ -196,9 +196,11 @@ fun TransactionManifest.executionSummary(encodedReceipt: BagOfBytes) =
 fun TransactionManifest.Companion.accountLockerClaim(
     lockerAddress: LockerAddress,
     claimant: AccountAddress,
-    claimableResources: List<AccountLockerClaimableResource>
+    claimableResources: List<AccountLockerClaimableResource>,
+    useTryDepositOrAbort: Boolean
 ) = manifestAccountLockerClaim(
     lockerAddress = lockerAddress,
     claimant = claimant,
-    claimableResources = claimableResources
+    claimableResources = claimableResources,
+    useTryDepositOrAbort = useTryDepositOrAbort
 )
diff --git a/jvm/sargon-android/src/test/java/com/radixdlt/sargon/TransactionManifestTest.kt b/jvm/sargon-android/src/test/java/com/radixdlt/sargon/TransactionManifestTest.kt
@@ -605,7 +605,7 @@ class TransactionManifestTest : SampleTestable<TransactionManifest> {
             instructionsString = openFile("account_locker_claim", "rtm").readText(),
             networkId = NetworkId.MAINNET
         )
-        val actualManifest = TransactionManifest.accountLockerClaim(
+        var actualManifest = TransactionManifest.accountLockerClaim(
             lockerAddress = LockerAddress.init("locker_rdx1drn4q2zk6dvljehytnhfah330xk7emfznv59rqlps5ayy52d7xkzzz"),
             claimant = AccountAddress.init("account_rdx128y6j78mt0aqv6372evz28hrxp8mn06ccddkr7xppc88hyvynvjdwr"),
             claimableResources = listOf(
@@ -629,7 +629,8 @@ class TransactionManifestTest : SampleTestable<TransactionManifest> {
                     resourceAddress = ResourceAddress.init("resource_rdx1n2ekdd2m0jsxjt9wasmu3p49twy2yfalpaa6wf08md46sk8dfmldnd"),
                     ids = listOf(NonFungibleLocalId.stringId("foobar"))
                 )
-            )
+            ),
+            useTryDepositOrAbort = false
         )
 
         assertEquals(expectedManifest.instructionsString, actualManifest.instructionsString)
@@ -639,6 +640,17 @@ class TransactionManifestTest : SampleTestable<TransactionManifest> {
             assertTrue(contains("claim_non_fungibles"))
             assertTrue(contains("account_rdx128y6j78mt0aqv6372evz28hrxp8mn06ccddkr7xppc88hyvynvjdwr"))
         }
+
+        actualManifest = TransactionManifest.accountLockerClaim(
+            lockerAddress = LockerAddress.init("locker_rdx1drn4q2zk6dvljehytnhfah330xk7emfznv59rqlps5ayy52d7xkzzz"),
+            claimant = AccountAddress.init("account_rdx128y6j78mt0aqv6372evz28hrxp8mn06ccddkr7xppc88hyvynvjdwr"),
+            claimableResources = emptyList(),
+            useTryDepositOrAbort = true
+        )
+
+        with(actualManifest.instructionsString) {
+            assertTrue(isEmpty())
+        }
     }
 
     private fun String.occurrences(substring: String): Int {