Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build many TXIntents for batch of TX applying Security Shield #349

Open
wants to merge 59 commits into
base: main
Choose a base branch
from

Conversation

CyonAlexRDX
Copy link
Contributor

@CyonAlexRDX CyonAlexRDX commented Jan 21, 2025

Jira ABW-4012

Note

Does not sign - SigningManager is to be implemented in future PR
Does not add any TXBatchQueue, the queue is to be implemented in future PR

Changes

Add ApplySecurityShieldCommitting implemented for SargonOS - using ApplyShieldTransactionsCommitter "manager". Input is a Vec of:

/// Will use Radix Engine toolkit to extract `entity_applying_shield: AddressOfAccountOrPersonaOrAccessController` from `manifest`
pub struct ManifestWithPayerByAddress {
    pub payer: Option<AccountAddress>,
    pub manifest: TransactionManifest,
    pub estimated_xrd_fee: Decimal,
}

estimated_xrd_fee hosts will have fetched when doing the preview.

Important

The API/DTOs passed between host and Sargon is non-final, this is just a first iteration. Furthermore we might expose more methods, e.g. logic for XRD balance validation when selecting payer, which this PR introduces code for. Once we are more sure about the API - from reviews, we will UniFFI export the needed APIs.

Steps

  • Builds many variants of manifest => many TransactionIntent, by
    • Map Address -> Entity by Profile lookup
    • Get XRD balances of XRD Vaults of Access Controllers and Accounts
    • For Securified entities => create 4 other variants of the manifest (noop for Unsecurified entities)
      • Modifies manifest: Adds lock fee
      • Conditionally depending on variant Modifies manifest: Adds XRD vault contribution (not done for manifests not doing Quick Confirm)
      • Fails if not enough balance
    • Build TransactionIntents for all of these applications (5 intents for securified entities) all using the same Epoch window (one week).
    • Persist notary private keys to be able to cancel transactions (to be implemented in future PR)
  • Signs (to be implemented in future PR)
  • Enqueue transactions (to be implemented in future PR)

Other minor

  • Add new address union type AddressOfVaultOrAccount (used by this PR)
  • Upgrade address union macro to impl TryFrom<$UNION> for $VARIANT
  • Upgrade existing Gateway API to fetch balance of Vaults
  • Add methods to Profile Logic crate for querying securified Accounts/Personas by their AccessControllerAddress
  • Change SecuredEntityControl to not contain an access_controller_address directly but rather addresses of a new type AddressesOfAccessController which contains access_controller_address and xrd_vault_address: VaultAddress.
  • Add missing TryFrom<Account> for SecurifiedAccount and vice versa for Securified/Persona.
  • Add missing TryFrom<Account> for UnsecurifiedAccount and vice versa for Unsecurified/Persona.

@CyonAlexRDX CyonAlexRDX changed the base branch from main to ac/wallet_interaction_applying_shield_abw-4056_part2 January 28, 2025 10:18
Copy link

codecov bot commented Jan 28, 2025

Codecov Report

Attention: Patch coverage is 88.27098% with 116 lines in your changes missing coverage. Please review.

Project coverage is 92.46%. Comparing base (e85b6da) to head (a5e2e68).

Files with missing lines Patch % Lines
...ateway/client-and-api/src/methods/state_methods.rs 43.58% 22 Missing ⚠️
...anifest_builder/xrd_vault_contribution_modifier.rs 67.16% 22 Missing ⚠️
...king_driver/support/test/mock_networking_driver.rs 54.83% 14 Missing ⚠️
...ation/without_intents/with_balance/unsecurified.rs 80.00% 12 Missing ⚠️
...ay/client-and-api/src/endpoints/state_endpoints.rs 60.00% 10 Missing ⚠️
...urity_shield/top_up_access_controller_xrd_vault.rs 63.15% 7 Missing ⚠️
...models/supporting-types/src/unsecurified_entity.rs 50.00% 4 Missing ⚠️
...ers/poly_manifest_builder/poly_manifest_builder.rs 93.75% 4 Missing ⚠️
...dresses/src/address/address_of_vault_or_account.rs 75.00% 3 Missing ⚠️
crates/core/utils/src/string_utils.rs 60.00% 2 Missing ⚠️
... and 10 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #349      +/-   ##
==========================================
- Coverage   92.64%   92.46%   -0.19%     
==========================================
  Files        1180      924     -256     
  Lines       26658    23986    -2672     
  Branches       77       77              
==========================================
- Hits        24698    22178    -2520     
+ Misses       1949     1797     -152     
  Partials       11       11              
Flag Coverage Δ
kotlin 97.63% <ø> (ø)
rust 92.16% <88.27%> (-0.15%) ⬇️
swift ?

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Base automatically changed from ac/wallet_interaction_applying_shield_abw-4056_part2 to main January 29, 2025 14:58
@CyonAlexRDX CyonAlexRDX force-pushed the ac/payloads_to_sign_for_applying_shield_abw-4012 branch from 1086b4c to d014fee Compare January 29, 2025 15:01
@CyonAlexRDX CyonAlexRDX changed the title [no ci] wip [WIP] Sign and submit batch of TX applying Security Shield Jan 30, 2025
@CyonAlexRDX CyonAlexRDX changed the title [WIP] Sign and submit batch of TX applying Security Shield sign and submit batch of TX applying Security Shield Jan 30, 2025
impl GetEntityAddressByAccessControllerAddress for Profile {
fn get_securified_entity_by_access_controller_address(
&self,
address: AccessControllerAddress,
Copy link
Contributor Author

@CyonAlexRDX CyonAlexRDX Feb 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will use RET to analyze the "apply shield" manifest and we will get a classification containing either an address of an Unsecurified Persona or an Unsecurified Account being securified or the address of an AcccessController of an already securified entity being updated - in case of the latter we will need to lookup the Account/Persona using that AccessControllerAddress

@CyonAlexRDX CyonAlexRDX marked this pull request as ready for review February 7, 2025 14:07
}
}

fn hacky_tmp_entities_applying_shield(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

everything named hacky_ will be removed once #373 is merged and we can get those addresses from the manifest using RETs analysis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants