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

feat: UOS Multipart #411

Merged
merged 15 commits into from
Oct 21, 2019
Merged

feat: UOS Multipart #411

merged 15 commits into from
Oct 21, 2019

Conversation

pmespresso
Copy link
Contributor

@pmespresso pmespresso commented Oct 9, 2019

#322

Starting with a utf8 message as this is most likely how a metadata blob will be transferred (#318). It would however be rather useful to add an extension range specifically for metadata payloads, although on the other hand metadata injection would probably only be with regard to Substrate based chains. @maciejhirsz?

Particularly because the UOS spec says that 7B is a forbidden first byte of any part_data, but that is how a metadata struct/json is going to be transfered.

  • - Multipart:
    • - Cold Signer MUST be able to start scanning the Multipart Payload at any frame.
    • - Cold Signer MUST NOT expect the frames to come in any particular order.
    • - Cold Signer SHOULD show a progress indicator of how many frames it has successfully scanned out of the total count.
    • - part_data MUST be stored by the Cold Signer, ordered by frame number, until all frames are scanned.
    • - Once all frames are combined, the part_data must be concatenated into a single binary blob, and then interpreted as a completely new albeit larger Payload, starting from the prefix table above.

ezgif-4-38c22cf26ef4

@pmespresso pmespresso marked this pull request as ready for review October 10, 2019 12:56
@pmespresso pmespresso changed the title wip: scanning progress and concat multipart to binary blob UOS Multipart Oct 11, 2019
Copy link
Contributor

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Could you add the instructions to test it? I've played with the current state of https://uostestqrs.herokuapp.com/ but it's a 26 frames (with long frame time) so I couldn't actually test it because it would take super long.

We somehow need to reset the isMultipart and/or frameCount at component unmount because say you start scanning a multipart, and you abort it for some reason (just like I did). You exit the scanner and get back to it, it'll still show the frame count and the "Multipart scanning, hold still.." although I'm not scanning anything any more.

src/util/decoders.js Outdated Show resolved Hide resolved
@Tbaut
Copy link
Contributor

Tbaut commented Oct 14, 2019

Thanks, please ping me for review when you think it's good to go.

@pmespresso
Copy link
Contributor Author

testing contract upload on Substrate so will need to uncomment the Substrate stuff before testing. it is 15 frames to upload incrementer contract.

@pmespresso
Copy link
Contributor Author

hi, @Tbaut the multipart pr is ready for review again. You can test with the random message in the herokuapp, and you can test with system.setCode with flipper.wasm or something in polkadot/apps too (beware, it'll be 13 frames)

@pmespresso
Copy link
Contributor Author

system.setCode(flipper.wasm)
ezgif-6-eab9d360ae99

@pmespresso pmespresso requested a review from Tbaut October 15, 2019 12:25
Copy link
Contributor

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

I tried the message without having the proper account on my phone and get this warning (because the account can't be found):
image

But more weirdly, I scanned a sudo transaction after this, and went back without signing it, then scanned the message multipart again, and now although I'm scanning the multipart (still without the account) I'm taken to the sudo tx confirmation screen.

Copy link
Contributor

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Message scanning is working well!
For contracts I get the unhandled promise rejection
image

or things like "unable to parse transaction":
image

@pmespresso
Copy link
Contributor Author

Hey @Tbaut thanks for all the testing! As we discussed yesterday, I've now archived the UOS-QRs repo and heroku app as it was originally a placeholder while the QR support was being added to apps.

From what I've tested, system.setCode with some small wasm file works on apps master, I'd appreciate if you tested on apps directly

@pmespresso
Copy link
Contributor Author

error codes for reference.

/// An invalid transaction validity.
#[derive(Clone, PartialEq, Eq, Encode, Decode, Copy)]
#[cfg_attr(feature = "std", derive(Debug, serde::Serialize))]
pub enum InvalidTransaction {
	/// The call of the transaction is not expected.
	Call,
	/// General error to do with the inability to pay some fees (e.g. account balance too low).
	Payment,
	/// General error to do with the transaction not yet being valid (e.g. nonce too high).
	Future,
	/// General error to do with the transaction being outdated (e.g. nonce too low).
	Stale,
	/// General error to do with the transaction's proofs (e.g. signature).
	BadProof,
	/// The transaction birth block is ancient.
	AncientBirthBlock,
	/// The transaction would exhaust the resources of current block.
	///
	/// The transaction might be valid, but there are not enough resources left in the current block.
	ExhaustsResources,
	/// Any other custom invalid validity that is not covered by this enum.
	Custom(u8),
}

tldr Invalid Transaction Payment = good, InvalidTransaction BadProof = bad

@pmespresso
Copy link
Contributor Author

Took a while to figure out what Invalid Trasaction: Custom(2) meant, but it means no permission since only these modules are allowed. https://github.com/paritytech/polkadot/blob/05b1c16802ab9e6dfc0e2542c69adf5bd8e7d729/runtime/src/lib.rs#L134

Copy link
Contributor

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Multipart seems to work well, logging some more related issues but we can merge this one in the mean time.
Signing and sending TX still works well (tested on Ehtereum and Dev Substrate)

@Tbaut Tbaut changed the title UOS Multipart Feat: UOS Multipart Oct 21, 2019
@Tbaut Tbaut changed the title Feat: UOS Multipart feat: UOS Multipart Oct 21, 2019
@Tbaut Tbaut merged commit 9f50388 into master Oct 21, 2019
@Tbaut Tbaut deleted the yj-multipart branch October 21, 2019 09:01
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.

2 participants