-
Notifications
You must be signed in to change notification settings - Fork 15
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
cardano-cli not in line with cip-0021 tag258 implementation - causing errors with hw wallets #567
Comments
A note on reasons for all/nothing approach to tags in HW wallets: Optionality in transaction serialization is costly for HW wallets because it adds to code size and leads to breaking changes in various low-level APIs. It also does not seem to serve any useful purpose to have some of the tags, but not others in a single tx --- it is said the tags will become mandatory in some future era. It would be best if cardano-cli allowed the user (via some command-line argument) to have control over the tags, at least an option to not include them at all, and to include all of the possible ones. (I don't see any other way to have it interoperable with who knows how many dapps and other tools that have a different release cycle.) |
This looks like an issue related with ledger format. @lehins can you pass it to your team please? |
@lehins any updates from your side? |
@gitmachtl thank you for reporting this issue, and for pinging me again about it. This is fixed in: IntersectMBO/cardano-ledger#4003 Whenever there is an optional new format in cbor cardano-cli/cardano-api and cardano-ledger should be using the new format by default for encoding. This is necessary to promote deprecation of the old format in the future. In this case the change is tag 258.
@janmazak, same can be said about all changes to serialization. It is costly for ledger too. However, we need to ensure backwards compatibility at least for an era or two. Because if we don't do this, the backwards incompatible changes (like addition of a 258 tag or change to a TxOut format like in Babbage, etc) will prevent older transactions from being deserializable in the new era.
This is technically infeasible.
I don't see how having a cli flag would solve this for dapps. The bottom line is there are two alternative formats that ledger supports: previous and the new. This is needed for all of the tooling that builds transactions to be able to transition to the new format. All of the software that requires to be able to decode transactions, like |
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 120 days. |
This issue was closed because it has been stalled for 120 days with no activity. Remove stale label or comment or this will be closed in 60 days. |
While i am testing HW-Wallet stuff for conway-era, i came across a wrong implementation of the new Tag(258) for conway transactions.
The decision was made - also in combination with HW-Wallets - to have the new Tag(258) everywhere or nowhere in the Tx. When generating a transaction for a dRep registration the output from cardano-cli looks like this example:
The Tag(258) was only added to the certificate part of the transaction.
Now after hw-cli takes over to use it for hw-witnessing, the corrected Tx looks like this:
So it added also Tag(258) for the first section (inputs) to be in line with CIP-0021 and the CDDL.
The witnesses are added to the Tx correctly with
cardano-cli conway transaction assemble
command, but caused an error at the end, because the Tx is now 3 bytes (the Tag258 itself) larger than the predicted Tx size for the fee calculation.So cardano-cli should respect the Tag(258) implementation according to CIP-0021/CDDL and use the Tags everywhere or nowhere so we have the correct Tx right from the beginning.
CIP-0021: cardano-foundation/CIPs#687
The text was updated successfully, but these errors were encountered: