Skip to content

feat(spark): support FIRO Spark verbose tx#2396

Merged
shamardy merged 5 commits intoGLEECBTC:devfrom
levoncrypto:spark_support
Apr 3, 2025
Merged

feat(spark): support FIRO Spark verbose tx#2396
shamardy merged 5 commits intoGLEECBTC:devfrom
levoncrypto:spark_support

Conversation

@levoncrypto
Copy link
Copy Markdown

This update introduces support for Spark verbose. The new integration ensures that the framework now supports Spark transaction parsing, expanding its capabilities for handling privacy-focused transactions.

The implementation maintains the core functionality while enhancing the framework’s versatility. I believe this addition will benefit users by providing a broader range of supported transactions within the Komodo Defi framework.

@laruh
Copy link
Copy Markdown

laruh commented Mar 20, 2025

first PR was closed due to some issues, this new one has been opened as a replacement

Copy link
Copy Markdown
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Thanks for re-opening the PR based on dev. Next review iteration!

@shamardy
Copy link
Copy Markdown
Collaborator

shamardy commented Mar 25, 2025

@levoncrypto There are still problems in serializations and deserializations which is why I assume in firo_spark_tx_details you didn't use get_tx_details_eq_for_both_versions because it fails as the v2 method serializes the hex instead of using the one from the verbose transaction and the serialization doesn't include this https://github.com/firoorg/firo/blob/bf7c7fa555bd00db970c5c3e64e7452ef20c800e/src/primitives/transaction.h#L389
You would need to add this to our Transaction struct as optional https://github.com/KomodoPlatform/komodo-defi-framework/blob/6f72dadb6979ecfe1bf27c0defa27f2f83ce78a1/mm2src/mm2_bitcoin/chain/src/transaction.rs#L210-L232
Then we need to handle this new field in the transaction serialization and deserialization code as it's handled here https://github.com/firoorg/firo/blob/bf7c7fa555bd00db970c5c3e64e7452ef20c800e/src/primitives/transaction.h#L327-L329 it would be good to validate that nType is in range to avoid deserializing firo transactions as other types. If you have any questions or need help with this, I would be happy to help :)

Change v_extra_payload as Option and deserialize call case for spark
Copy link
Copy Markdown
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Thank you for the fixes! A few more notes :)

Q: Do we need to set this extra payload for transactions generated by KDF or is it not needed for standard transactions.

@levoncrypto
Copy link
Copy Markdown
Author

Extra payload need only for special transactions.

Copy link
Copy Markdown
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Amazing Work! Will wait for CI to finish before merging!

@shamardy shamardy merged commit 4be0ec7 into GLEECBTC:dev Apr 3, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants