feat(eip712): python client for the device-driven walk - #48
Merged
Conversation
The third implementation of the same protocol, and the point of it is that there are now three: firmware C, hdwallet TypeScript, and this. Two implementations built to one spec can share a misreading and agree with each other forever; a third that disagrees turns that into a test failure. Mirrors packages/hdwallet-keepkey/src/eip712Streaming.ts deliberately, function for function, so a divergence shows up as a failing test in one of them rather than as a bad signature in the field. Verified against the TS behaviour: uint256 max -> ff * 32 (the unlimited approval the old path refused) int16 -2 -> fffe (two's complement at the declared width) uint0256 -> refused, "Non-canonical integer width" uint256[0] -> refused, "Malformed array dimension" uint -> refused, "Integer type must state its width" bytes032 -> refused, "Non-canonical bytes width" Bindings regenerated with the PINNED protoc 3.5.1 in kktech/firmware:v8, the way build_pb.sh does it, producing old-style _descriptor.FileDescriptor output. Not with a modern protoc: that produced AddSerializedFile bindings that the Alpine 3.8 / Python 3.6 CI container cannot load, and it broke every alpha run until it was reverted. Two notes for whoever runs this next: - the image's `python` is Python 2 and has protobuf; `python3` does not. Install it explicitly. - protobuf 3.20.3 is NOT available for that image's python3 -- the index tops out at 4.21.0rc2 with 3.19.6 the last usable 3.x. Anything pinning 3.20.3 will fail to resolve.
Four cases against real firmware in the emulator, all passing. The one that matters: the device's own domainSeparator and messageHash for the canonical Mail/Person document equal the values published by assets/eip-712/Example.js in ethereum/EIPs. domainSeparator f2cee375fa42b42143804025fc449deafd50cc031ca257e0b194a650a912090f messageHash c52c0ee5d84264471806290a3f2c4cecfc5490626bf912d01f240d7a274b371e Both numbers come from OUTSIDE this repository, and that is the whole point. The firmware C, the hdwallet TypeScript and the python client were written by one hand against one reading of the spec, so three of them agreeing proves the reading is self-consistent and nothing more. A shared misreading would produce three consistent WRONG answers. It cannot produce these two. It also exercises the nested-struct path for real: Mail references Person twice, so the walk pushes a child frame, derives Person's typeHash through its own closure, folds it to 32 bytes and hands it back to the parent -- machinery that until now had only been reasoned about. Forty round trips. The other three: - an array of structs walks end to end. Arrays hash WITHOUT a typeHash prefix, so getting that wrong yields a digest no verifier reproduces rather than an error anyone would notice. - a fixed dimension must match the document. It is part of the type string and therefore of typeHash, and the device only ever learns the count from us -- accept a different one and it signs a document whose type declares another, with nothing downstream able to tell. - AdvancedMode gates the endpoint. The walk helper answers only what the device asks, in the order it asks. The host chooses nothing, which is the property under test as much as the hashes are.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The third implementation of the same protocol, and that is the point: firmware C, hdwallet TypeScript, and this.
Two implementations built to one spec can share a misreading and agree with each other forever. A third that disagrees turns that into a test failure — the same reason the typeHash tests now assert published literals instead of a string written in the test.
Mirrors
eip712Streaming.tsfunction for function. Verified to match:Bindings regenerated with the pinned protoc 3.5.1 in
kktech/firmware:v8, asbuild_pb.shdoes — old-style_descriptor.FileDescriptoroutput. Not a modern protoc: that producesAddSerializedFilebindings the Alpine 3.8 / Python 3.6 CI container cannot load, and it broke every alpha run until reverted.Two notes for whoever runs this next:
pythonis Python 2 and has protobuf;python3does not — install it explicitly