Account locker in manifest summary encountered entities

[sergiupuhalschi-rdx]

No description provided.

[sergiupuhalschi-rdx]

I'm not sure about the enum name

[CyonAlexRDX]

Did you ask Omar if there is any other address which belongs here?

[sergiupuhalschi-rdx]

No, I didn't ask, but should we add other types even if we don't need them in the wallets right now? It will be quite easy to add a new address type when required.

[GhenadieVP]

Let's not add other types for now, the logic in the Wallet to display the "used dApps" is explicitly for globalGenericComponents + account locker.

At some point, RET had a field called userApplications in summary, meant to point that these potentially have associated dApps. Maybe we can have the field in the execution summary more specific to potential dApps used, rather than generic ManifestEncounteredAddress, which presumably, by the name, should include all addresses

[sergiupuhalschi-rdx]

@GhenadieVP do you have an example? I agree, but I still can't think of a better name to reflect what you described, UserApplicationAddress doesn't sound better and it lacks context (as the keyword manifest offers).

[GhenadieVP]

Isn't it too generic? I mean that we are filtering the original addresses after all

[sergiupuhalschi-rdx]

At first, I thought that AddressReferencedInTX is more specific, but now that I think of it all of the addresses referenced in a manifest are referenced in the transaction so I guess you're right and it's better to put emphasis on the fact that the addresses are of components. I'll rename it to ManifestEncounteredComponentAddress.

[CyonAlexRDX]

but it is not component addresses. LockerAddress is not ComponentAddress right?

[sergiupuhalschi-rdx]

Yes, it's not a component address, but locker is a component, right? But I see what you're saying and that's why I didn't use component when naming it initially.

[sergiupuhalschi-rdx]

Now the component keyword expresses that it's not just encountered addresses, but addresses of components (not resources or other types of addresses there are). I'd leave it as is unless someone has other suggestions that reflect the fact that the addresses were filtered to match this criteria.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.4%. Comparing base (9c874a3) to head (354e519).
Report is 1 commits behind head on main.

Additional details and impacted files

@@          Coverage Diff          @@
##            main    #213   +/-   ##
=====================================
  Coverage   97.4%   97.4%           
=====================================
  Files        932     938    +6     
  Lines      14954   15005   +51     
  Branches      64      66    +2     
=====================================
+ Hits       14572   14623   +51     
  Misses       375     375           
  Partials       7       7           

Flag	Coverage Δ
kotlin	98.8% <100.0%> (+<0.1%)	⬆️
rust	96.9% <100.0%> (+<0.1%)	⬆️
swift	99.4% <100.0%> (+<0.1%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...ss/ManifestEncounteredAddress+Wrap+Functions.swift	100.0% <100.0%> (ø)
...ddress/ManifestEncounteredAddress+Swiftified.swift	100.0% <100.0%> (ø)
crates/sargon/src/core/error/common_error.rs	100.0% <ø> (ø)
...ic_types/manifest_encountered_component_address.rs	100.0% <100.0%> (ø)
...t/low_level/execution_summary/execution_summary.rs	100.0% <100.0%> (ø)
..._summary/transaction_manifest_execution_summary.rs	100.0% <ø> (ø)
.../extensions/ManifestEncounteredComponentAddress.kt	100.0% <100.0%> (ø)

... and 3 files with indirect coverage changes

[CyonAlexRDX]

Good job!

Missing sample values. Can you also Swift and Kotlinify this.

In Swift conform to Address protocol, see https://github.com/radixdlt/sargon/blob/main/apple/Sources/Sargon/Extensions/Swiftified/Address/AddressOfAccountOrPersona%2BSwiftified.swift

[CyonAlexRDX]

LGTM ! Good job

[CyonAlexRDX]

We can try this.

But it is possible this breaks test Swift - and possible it works for most runs but fails for some due to non determinism.

If we have troubles we can change so that Kotlin tests are run AFTER Swift tests:

Which can be done by using needs: test-Swift