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

Moving inner_tx from WrapperTx to Tx. #1093

Merged
merged 12 commits into from
Jun 5, 2023
Merged

Moving inner_tx from WrapperTx to Tx. #1093

merged 12 commits into from
Jun 5, 2023

Conversation

murisi
Copy link
Contributor

@murisi murisi commented Jan 27, 2023

The objective is to (with as little change as possible) make Namada transactions signable on hardware constrained wallets by making them smaller. The approach to this taken in this branch is to replace inner transactions (amongst other objects) with their hashes, and then only attach the encrypted payloads at the top level.

More precisely, the following changes have been made:

  • Replaced the transaction code field of Tx with a sum type that can either contain a hash or the literal code
  • Removed the inner_tx field of WrapperTx leaving behind tx_hash
  • Added an inner_tx field to Tx representing an attached encrypted inner transaction
  • Added an inner_tx_code field to Tx representing an attached encrypted inner transaction code
  • The ledger now decrypts and checks the attached inner_tx field against the Wrapper::tx_hash field
  • The ledger now decrypts and checks the attached inner_tx_code field against the hash in the Tx::code field of the inner transaction

The implications of these changes are:

  • Clients can now sign over either the full WASM code or a hash of it
  • No encryption is required for/before the signing process (and hence on hardware wallets) because the signature for a WrapperTx is over the hash of an unencrypted Tx

@murisi murisi force-pushed the murisi/signable-txs branch 2 times, most recently from 5a0c9ff to d3b4848 Compare January 27, 2023 10:19
@murisi murisi force-pushed the murisi/signable-txs branch 2 times, most recently from 4b4b165 to 9b28738 Compare January 31, 2023 05:43
@murisi murisi force-pushed the murisi/signable-txs branch 2 times, most recently from c281592 to e7f1e42 Compare January 31, 2023 15:49
@murisi
Copy link
Contributor Author

murisi commented Feb 6, 2023

pls update wasm

@murisi murisi marked this pull request as ready for review February 6, 2023 09:54
@murisi
Copy link
Contributor Author

murisi commented Feb 6, 2023

pls update wasm

core/src/proto/types.rs Outdated Show resolved Hide resolved
core/src/types/internal.rs Outdated Show resolved Hide resolved
apps/src/lib/node/ledger/shell/mod.rs Show resolved Hide resolved
@Fraccaman
Copy link
Member

pls spawn devnet [anoma-devnet-0.13.0-zondax.toml,60,heliaxdev@1be9883,ON]

@Fraccaman
Copy link
Member

pls spawn devnet [anoma-devnet-0.13.0-zondax.toml,60,heliaxdev@1be9883,ON]

@Fraccaman
Copy link
Member

pls spawn devnet [anoma-devnet-0.13.0-zondax,60,heliaxdev@1be9883,ON]

@github-actions
Copy link
Contributor

Devnet with chain id internal-devnet-2ba.40222721e7 spawned succesfully!

@Fraccaman
Copy link
Member

pls spawn devnet [anoma-devnet-0.13.0-zondax,60,heliaxdev@1be9883,ON]

@github-actions
Copy link
Contributor

Devnet with chain id internal-devnet-7c7.92696bafc8 spawned succesfully!

@Fraccaman
Copy link
Member

pls spawn devnet [anoma-devnet-0.13.0-zondax,60,heliaxdev@1be9883,ON]

@github-actions
Copy link
Contributor

Devnet with chain id internal-devnet-30a.875b15da08 spawned succesfully!

@tzemanovic
Copy link
Member

@murisi would we still want to use this with the planned changes in #1255? I also see #1156, is that an alternative to this one?

@murisi
Copy link
Contributor Author

murisi commented Apr 7, 2023

@murisi would we still want to use this with the planned changes in #1255? I also see #1156, is that an alternative to this one?

Yes, that's the plan. I'm working on a branch that applies the proposal #1255 to this branch. If this idea works out, then this PR will be superseded/redundant. #1156 was the original approach to enabling hardware wallet signing and is now superseded/redundant. I've just closed it now.

@tzemanovic tzemanovic added the held from draft draft ready, but something is holding it back label Apr 11, 2023
@cwgoes cwgoes mentioned this pull request May 18, 2023
10 tasks
@Fraccaman Fraccaman merged commit 30aec50 into main Jun 5, 2023
@Fraccaman Fraccaman deleted the murisi/signable-txs branch June 5, 2023 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
held from draft draft ready, but something is holding it back
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants