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

Murisi/shared masp2 #848

Closed
wants to merge 9 commits into from
Closed

Murisi/shared masp2 #848

wants to merge 9 commits into from

Conversation

murisi
Copy link
Contributor

@murisi murisi commented Nov 30, 2022

Factored the platform specific components out of ShieldedContext and then moved ShieldedContext into the shared crate. Essentially addressing: #302 .

@@ -18,6 +18,52 @@ use masp_primitives::transaction::{
};
use masp_proofs::sapling::SaplingVerificationContext;

use std::collections::hash_map::Entry;
Copy link
Member

Choose a reason for hiding this comment

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

if think we could move the additions into a sub-module and re-export here what's needed


async fn query_epoch(&self) -> Epoch;

fn local_tx_prover(&self) -> LocalTxProver;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is LocalTxProver specific to the consumer? Something that would differ from what is being done on desktop/CLI?

Comment on lines +267 to +275
async fn query_conversion(
&self,
asset_type: AssetType,
) -> Option<(
Address,
Epoch,
masp_primitives::transaction::components::Amount,
MerklePath<Node>,
)>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Also here, does this mean that we should define this in a web/WASM specific way?

    async fn query_conversion(
        &self,
        asset_type: AssetType,
    ) -> Option<(
        Address,
        Epoch,
        masp_primitives::transaction::components::Amount,
        MerklePath<Node>,
    )>;

@cwgoes
Copy link
Contributor

cwgoes commented Jan 16, 2023

@murisi @mariari is this PR still applicable?

@mariari
Copy link
Member

mariari commented Jan 16, 2023

@murisi @mariari is this PR still applicable?

The work in #925 (integration branch) / #921 (branch proper) follows this work up given the commit log @murisi would know more about the work committed after I went on break.

@murisi
Copy link
Contributor Author

murisi commented Jan 22, 2023

@murisi @mariari is this PR still applicable?

@mariari is correct. The initial scope of my work was to put the MASP transaction code into an SDK. After completing that, the scope then broadened into putting CLI subcommands into an SDK. Hence this branch is followed up/superseded by #925 and #921 .

@murisi murisi closed this Jan 22, 2023
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.

None yet

5 participants