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

WIP - Modify manifests based on Security State / Fee Payer / Guarantees #384

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

micbakos-rdx
Copy link
Contributor

@micbakos-rdx micbakos-rdx commented Feb 19, 2025

Warning

This PR is still in early WIP
I am publishing it just to share the progress so far and keep track of some significant changes that need to happen. One example is the need, IMHO, to hoist the modifying of the manifests to sargon os. Currently this responsibility is in hosts and does not involve the state of the profile.

I will type in a full description of the summary of the work, when I will finalise the design. For now I am going to keep track of the changes:

Significant changes

  • Realised that hosts don't edit subintent manifests to assert guarantees. So the logic that adds guarantees needs to be shared to subintents too.
  • That means that internally to the manifests crate I needed to define some traits with common properties with different versions of manifests, instructions and builders. Thus I created the IntoManifest, IntoInstruction and IntoManifestBuilder traits which can be used when modifying any kind of manifest. These traits are pub(crate) since they are only useful to this crate which has to do with manifests and modifying them.
  • A modify method on manifests that can
    1. add proofs for securified entities,
    2. add lock fee and add proof for that if the fee payer is securified,
    3. attach guarantees (possibly increasing the offset by 1 or 2 if fee payer exists and if that fee payer is not included in the original summarised manifest but is securified)
  • Currently those manifest modifications are exposed as separate functions without the context of sargon os. That is because the host used to take the decision on what to change to the manifest. Since sargon os can check the status of the entities involved in the transaction, can decide which proofs to add and just expose to the hosts the inputs that the user chooses, like the fee payer, or the fee amount (currently calculated in hosts).
  • By doing that it makes it really easy to modify the manifest of the tx when a shield is applied to an entity (we need to top up the AC with constant amount of XRD), or when a recovery tx takes place (we need to top up the AC with the fees that are going to be deducted by the current tx). In order to achieve that we need to apply another instruction. So sargon os can play a significant role here.
  • In order to do all that I first need to move the StaticallyAnalyzableManifest and DynamicallyAnalyzableManifest into manifests crate. The reason for that is that modifying the manifests we are about to analyze, requires the dependency into manifests from manifest-models. This is impossible due to cyclic dependencies.

@micbakos-rdx micbakos-rdx self-assigned this Feb 19, 2025
@micbakos-rdx micbakos-rdx changed the title [WIP] Modify manifests based on Security state / Fee Payer / Guarantees [WIP] Modify manifests based on Security State / Fee Payer / Guarantees Feb 19, 2025

This comment was marked as outdated.

@micbakos-rdx micbakos-rdx force-pushed the mib/ABW-4164-modify-tx-manifest branch from 88c1873 to c6016f4 Compare February 21, 2025 15:06
@micbakos-rdx micbakos-rdx force-pushed the mib/ABW-4164-modify-tx-manifest branch from 825bd75 to b4535f5 Compare February 26, 2025 16:03
@micbakos-rdx micbakos-rdx changed the title [WIP] Modify manifests based on Security State / Fee Payer / Guarantees WIP - Modify manifests based on Security State / Fee Payer / Guarantees Feb 26, 2025
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.

1 participant