Skip to content

Conversation

AngelCastilloB
Copy link
Member

Context

We had a round-trip issue in our hardware wallet output mappings. This PR introduces a change to check for the actual format on the raw CBOR and passes as as context to the type mappers

Proposed Solution

Analyze the raw CBOR to detect the output format and pass it as part of the transformation context.

@AngelCastilloB AngelCastilloB changed the title Fix/output mapping on hardware wallets [LW-12624] Fix output mapping on hardware wallets Apr 11, 2025
@AngelCastilloB AngelCastilloB changed the title [LW-12624] Fix output mapping on hardware wallets LW-12624: Fix output mapping on hardware wallets Apr 11, 2025
*
* @param outputs The outputs to be verified.
*/
hasBabbageOutput(outputs: Array<Serialization.TransactionOutput>): boolean {
Copy link
Member Author

Choose a reason for hiding this comment

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

We can probably make this a method on the TransactionBody class

Copy link
Member

Choose a reason for hiding this comment

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

Good idea, it would deduplicate this

Copy link
Member Author

Choose a reason for hiding this comment

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

done here ab26ecf and here b20f328 and here 2135507

mkazlauskas
mkazlauskas previously approved these changes Apr 11, 2025
Copy link
Member

@mkazlauskas mkazlauskas left a comment

Choose a reason for hiding this comment

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

Great work! 🕵️

@rhyslbw
Copy link
Member

rhyslbw commented Apr 11, 2025

Failing test

@AngelCastilloB AngelCastilloB force-pushed the fix/output-mapping-on-hardware-wallets branch from a2b3c5c to 2135507 Compare April 11, 2025 11:36
@AngelCastilloB
Copy link
Member Author

Failing test

fixed in b20f328

@rhyslbw rhyslbw self-requested a review April 11, 2025 12:52
Copy link
Member

@rhyslbw rhyslbw left a comment

Choose a reason for hiding this comment

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

Great work @AngelCastilloB

@AngelCastilloB AngelCastilloB force-pushed the fix/output-mapping-on-hardware-wallets branch from 2135507 to 3de9a1d Compare April 14, 2025 04:44
@AngelCastilloB AngelCastilloB merged commit a7c5b18 into master Apr 14, 2025
9 of 10 checks passed
@AngelCastilloB AngelCastilloB deleted the fix/output-mapping-on-hardware-wallets branch April 14, 2025 05:18
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.

3 participants