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

Add unpack command #433

Merged
merged 1 commit into from Jun 11, 2020
Merged

Conversation

yeastplume
Copy link
Member

  • Adds a grin-wallet unpack command, which unpacks a slatepack message, decrypts (if it's intended for this wallet), and displays the slate in JSON (if possible)
  • Cleans up some of the command-line help text to reflect Slatepack changes

@@ -90,7 +90,9 @@ impl<'a> Slatepacker<'a> {
};

slatepack.ver_check_warn();
slatepack.try_decrypt_payload(self.0.dec_key)?;
if decrypt {
slatepack.try_decrypt_payload(self.0.dec_key)?;
Copy link
Member

Choose a reason for hiding this comment

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

If I understand correctly when calling the unpack API with decrypt true and if the slatepack is not encrypted that will fail?
Why not:

  1. Try to decrypt by default without failing in a specific context.
  2. Do not return an error when unpacking with true and simply failover false case.

Copy link
Member

Choose a reason for hiding this comment

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

Nevermind missed

	pub fn try_decrypt_payload(&mut self, dec_key: Option<&edSecretKey>) -> Result<(), Error> {
		if self.mode == 0 {
			return Ok(());
		}

Copy link
Member

@quentinlesceller quentinlesceller left a comment

Choose a reason for hiding this comment

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

Looking good testing it now. What is the rationale for having an additional decrypt arg instead of trying to decrypt it by default?

@quentinlesceller
Copy link
Member

Tested and everything looks fine.

@yeastplume
Copy link
Member Author

Looking good testing it now. What is the rationale for having an additional decrypt arg instead of trying to decrypt it by default?

Just that the logic flow was set up in most places to assume the decode function also handled decryption, was much easier to provide a flag to that function to allow it to handle the case where you just want the contents without decrypting than changing the flow everywhere.

@yeastplume
Copy link
Member Author

Tested and everything looks fine.

Great, thanks for testing!

@yeastplume yeastplume merged commit 5e20f5f into mimblewimble:master Jun 11, 2020
@yeastplume yeastplume deleted the unpack_cmd_line branch July 13, 2020 10:20
antiochp pushed a commit to antiochp/grin-wallet that referenced this pull request Aug 7, 2020
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

2 participants