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/namada integration #44

Merged
merged 15 commits into from
May 25, 2023
Merged

Murisi/namada integration #44

merged 15 commits into from
May 25, 2023

Conversation

murisi
Copy link
Contributor

@murisi murisi commented Apr 15, 2023

Made some changes to this MASP crate in order to facilitate integration with the Namada crate. More specifically, the changes are as follows:

  • Added common traits (i.e. Borsh (de)serialization, Clone, Debug, and PartialOrd) to various objects in cases where they are required by the Namada crate
  • Removed the js feature from the getrandom dependency of the masp_proofs crate as this disrupts the compilation of wasm32-unknown-unknown modules in the Namada crate
  • Made the transaction Builder object serializable in order to use it to store/capture the auxiliary inputs that were used to construct a Transaction. Having this may be useful for hardware wallets validating a Transaction.
  • Added a missing function to the Builder interface in order to allow for the adding of convert descriptions to a Transaction.
  • Now re-export some cryptographic crates so that they can be used from the Namada crate.
  • Changed TransparentAddress to be a byte array (that can be used to store arbitrary hashes) because requiring a secp256k::PublicKey was too constraining for the Namada crate which represents Addresses with PublicKeyHashes.
  • Actually add input TransparentAddresses to the authorized TxIns and include them in the TXID digest. This could be useful for enabling more stringent validation of Tx sections in Namada or for combining shielded transactions with non-Transfers.
  • Removed the secp256k dependency as it would be redundant after the above changes.

See the Namada crate using this branch here: anoma/namada#1280

@murisi murisi requested a review from a user April 15, 2023 16:19
@murisi murisi force-pushed the murisi/namada-integration branch from 8b8e20c to 6a7c0eb Compare April 15, 2023 18:18
@ghost
Copy link

ghost commented Apr 16, 2023

What is the plan for getrandomon the wasm backend if we don't use the js feature?
Do the wasm modules in Namada need the MASP crates? And if they do need MASP, then what are they doing with MASP that doesn't require getrandom?

@murisi murisi force-pushed the murisi/namada-integration branch from 8998017 to 3fee08a Compare April 18, 2023 07:56
@murisi
Copy link
Contributor Author

murisi commented Apr 19, 2023

Hi! Good points you bring up.

Do the wasm modules in Namada need the MASP crates?

Indirectly, yes. The WASM modules depend on the namada crate which depends on masp_proofs. Though using feature flags and ifdefs in the code to prevent this dependency from happening would be ideal, removing the js feature just seemed to be the most straightforward solution for now.

What is the plan for getrandomon the wasm backend if we don't use the js feature?

Removal of the js feature from the masp_proofs crate doesn't necessarily prevent the crate from being used in js environments. Since rust crate features are additive, consumers can independently/externally enable js in this crate by adding getrandom = { version = "0.2", features = ["js"] } directly into their crate. (I believe this is what we are doing for the web interface, see: https://github.com/anoma/using-namada-sdk-poc/blob/main/usage-of-namada-sdk/Cargo.toml#L31 .) Hence taking this removal approach has the benefit of allowing the masp_proofs crate to be used in both js and non-js/custom environments.

@murisi murisi force-pushed the murisi/namada-integration branch from 1ed99e8 to cfea8c9 Compare May 19, 2023 07:09
.map(|txin| TxIn {
asset_type: txin.asset_type,
address: txin.address,
transparent_sig: f.map_script_sig(txin.transparent_sig),
Copy link

Choose a reason for hiding this comment

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

what is the purpose of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This map_authorization function is no longer required - I'll remove it.

Formerly it was used in order to achieve shielded transaction validation. To be precise calling signature_hash on TransactionData required that TransparentAuthorizingContext be implemented for the authorization of its transparent bundle. Unfortunately since TransparentAuthorizingContext was only implemented for Unauthorized, I had ended up mapping authorized bundles to unauthorized bundles before calling signature_hash. I no longer do this because the client now creates unauthorized bundles from asset type, address, and value triples by more direct means.

@murisi murisi merged commit 0d7dc07 into main May 25, 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.

1 participant