Vasil branch produces incorrect witnesses

[TotallyNotChase]

It looks like the vasil branch of nami properly understands babbage era transaction (at least, the ones that have all the babbage era features set to use the *None constructors from cardano-api) - however, I believe it creates the witnesses improperly, as subsequent transaction submission towards testnet yields:

ShelleyTxValidationError ShelleyBasedEraBabbage (ApplyTxError [UtxowFailure (FromAlonzoUtxowFail (WrappedShelleyEraFailure (InvalidWitnessesUTXOW [VKey (VerKeyEd25519DSIGN ...)])))])

When I try to manually sign with a private key using cardano-serialization-lib 11.0.0-rc.1, with effectively the same logic nami uses under the hood - everything works fine and the transaction goes through.

Since the issue probably lies in the cardano-multiplatform-lib wasm files used by nami, I'm unsure where exactly to look to try and quickly fix it via a PR.

[TotallyNotChase]

If the CML being used is this one, then the issue should be in serialization.rs. CSL does it differently https://github.com/Emurgo/cardano-serialization-lib/commit/4a904d1525b4edf66879e4b1e520a0e35adc7810#diff-4b90653e5bfcd7f31fabfc9cab5f8f5a77db63a4ad3338090026fa20a00dd9b1R1820-R1824

On that note, is there a particular reason for using a fork of a fork (CML) rather than CSL? CSL seems to have full vasil support already, which seems to work properly in several testers' (including myself) experience. Maintaining a fork of a fork is understandably more difficult, perhaps the README could explain what exactly CSL is missing for nami (and lucid) to have made such a decision?

[alessandrokonrad]

It looks like the vasil branch of nami properly understands babbage era transaction (at least, the ones that have all the babbage era features set to use the *None constructors from cardano-api) - however, I believe it creates the witnesses improperly, as subsequent transaction submission towards testnet yields:

ShelleyTxValidationError ShelleyBasedEraBabbage (ApplyTxError [UtxowFailure (FromAlonzoUtxowFail (WrappedShelleyEraFailure (InvalidWitnessesUTXOW [VKey (VerKeyEd25519DSIGN ...)])))])

When I try to manually sign with a private key using cardano-serialization-lib 11.0.0-rc.1, with effectively the same logic nami uses under the hood - everything works fine and the transaction goes through.

Since the issue probably lies in the cardano-multiplatform-lib wasm files used by nami, I'm unsure where exactly to look to try and quickly fix it via a PR.

Nami only generates vkey witnesses for you. The TransactionWitnessSet hasn't changed at all. Only the plutus v2 script option was added as additional entry in the map. But that doesn't affect the serialization really. Also if you take a look at dcSparks implementation: https://github.com/dcSpark/cardano-multiplatform-lib/blob/a675360fbcfbcafde8e1dae95e17d36294ab2d59/rust/src/serialization.rs#L1972-L2005 It's how I do it, I'm not sure why Emurgo is doing it differently, but in the end it achieves the same.

Maintaining a fork of a fork is understandably more difficult, perhaps the README could explain what exactly CSL is missing for nami (and lucid) to have made such a decision?

Well there are quite a few things: Bad Coin selection algorithm, it doesn't fully support plutus (doesn't take redeemers into account for the min fee calculation), poor transaction builder (no auto collateral balancer, you cannot add add plutus scripts to anything else than inputs, doesn't let you build a full plutus tx including automatic fee calculation), some other bugs.

[TotallyNotChase]

Hmmm, weird - I don't quite understand where else the problem could lie if it's not where I originally suspected it to be. I can sign and submit this transaction just fine with a manually written signTx function using CSL, but I cannot submit it after signing via nami.

Well there are quite a few things: Bad Coin selection algorithm, it doesn't fully support plutus (doesn't take redeemers into account for the min fee calculation), poor transaction builder (no auto collateral balancer, you cannot add add plutus scripts to anything else than inputs, doesn't let you build a full plutus tx including automatic fee calculation), some other bugs.

Do you think there was ever a possibility to fix these bugs one by one and PR them up? Or is the rationale behind it is simply that it would take too much time?

[alessandrokonrad]

Hmmm, weird - I don't quite understand where else the problem could lie if it's not where I originally suspected it to be. I can sign and submit this transaction just fine with a manually written signTx function using CSL, but I cannot submit it after signing via nami.

It's possible that my version of the CSL deserializes the tx a bit differently then the the CSL from Emurgo and so the signature is different in the end. Could you send me a sample tx in cbor which you created with the CSL and tried to sign with Nami?

[TotallyNotChase]

The core transaction cbor is produced via a Haskell server using official cardano-node. I would share a CBOR hex of such a transaction but it couldn't be signed without my private key anyway - but I think any properly produced simple transaction should suffice. This particular transaction was a Tx BabbageEra, in terms of cardano api types.

But more importantly, I think I see what you mean here. This tx CBOR passes through the frontend and that requires the help of either CSL/CML. Everything was originally using dcSpark's CML - but the latest release didn't have vasil support (it still doesn't have a proper NPM release for said support!). So the first error I encountered was obviously a deserialization failure on the tx CBOR way before it hit nami.

I noticed Emurgo's CSL already had a proper release with "full vasil support" and it was conveniently dependable through npm and the like. So I used that for the time being, the next deserialization failure was on nami - which is understandable since I was using the mainline version, not the one in vasil branch. So I switched it out to use the nami from vasil branch. And the rest is summarized above.

This is the gist of the CSL + nami code I used at the time:

import * as CSL from '@emurgo/cardano-serialization-lib-browser'; // 11.0.0-rc.1

const txCborHex = ...; // A valid babbage era tx CBOR

const transaction = CSL.Transaction.from_bytes(hexToByteArray(txCborHex));
const transactionWitnessSet = transaction.witness_set();
const transactionBody = transaction.body();
const transactionAuxiliaryData = transaction.auxiliary_data();

let txVkeyWitnesses = await nami.signTx(txCborHex, true);
txVkeyWitnesses = CSL.TransactionWitnessSet.from_bytes(
    hexToByteArray(txVkeyWitnesses)
);

// add the sig to the base tx witness set
transactionWitnessSet.set_vkeys(txVkeyWitnesses.vkeys());

// compose a new tx
const readyToSubmit = CSL.Transaction.new(transactionBody, transactionWitnessSet, transactionAuxiliaryData);

// convert to cbor hex.  you can submit this in the browser with cardano.submitTx() or using cli on the backend
const finalTx = byteArrayToHex(readyToSubmit.to_bytes());

const txHash = await nami.submitTx(finalTx);

console.log("Tx submitted: " + txHash);

Your explanation does make sense now I reckon. Since I wasn't using the exact same cardano compat wasm as nami. Though it's not like I could do it easily in the first place - there's no proper release with vasil support for official CML yet, unsure if your fork of CML has one. (this is why fragmentation like this hurts the users - I'm sure you'd agree)

[alessandrokonrad]

I would share a CBOR hex of such a transaction but it couldn't be signed without my private key anyway

The idea was to simply check if Nami would change the cbor representation after deserializing and serializing the body again to sign it.

Your explanation does make sense now I reckon. Since I wasn't using the exact same cardano compat wasm as nami. Though it's not like I could do it easily in the first place - there's no proper release with vasil support for official CML yet, unsure if your fork of CML has one. (this is why fragmentation like this hurts the users - I'm sure you'd agree)

Yeah sure I agree and it's kind of a mess.

[TotallyNotChase]

Indeed, it is the incompatibility between the cardano libs. Using the specific cardano multiplatform lib impl from temporary_modules in this repo (vasil branch, of course) works properly. I tried with newer official dcSpark/CML as well since I noticed there was a 1.0.0 release - same issue there.

This is rather weird though - surely incompatibility with the upstream stuff isn't intentional. After all, most frameworks built around this stuff that I've worked on, are and will be using either official CSL or official CML. I suppose it's worth a deeper look to find the source of incompatibility.

[alessandrokonrad]

Indeed, it is the incompatibility between the cardano libs. Using the specific cardano multiplatform lib impl from temporary_modules in this repo (vasil branch, of course) works properly. I tried with newer official dcSpark/CML as well since I noticed there was a 1.0.0 release - same issue there.

This is rather weird though - surely incompatibility with the upstream stuff isn't intentional. After all, most frameworks built around this stuff that I've worked on, are and will be using either official CSL or official CML. I suppose it's worth a deeper look to find the source of incompatibility.

I'm pretty sure it has to do with how the outputs are serialized. There is a new format, but the legacy format is the primary one supported by the CSL.

[TotallyNotChase]

Hello again @alessandrokonrad

Some more thoughts: I suspect this affects Lucid as well. I can't use Lucid's vasil branch with other wallets (for example: eternl) - only nami. Other wallets are also failing submission with a similar error of sorts. Do you reckon this issue is worth a deeper look? I could perhaps try helping if you want me to look in a particular spot. I'd ideally like this incompatibility fixed since this doesn't only affect nami adoption on our (and our clients') ecosystem side, but it may now also affect Lucid adoption - I'd like to prevent that.

Perhaps I should start by looking at the tx output serialization. Though the error in different wallets (and cardano submit api) indicate it's an issue in the signatories for some reason.

[alessandrokonrad]

Hello again @alessandrokonrad

Some more thoughts: I suspect this affects Lucid as well. I can't use Lucid's vasil branch with other wallets (for example: eternl) - only nami. Other wallets are also failing submission with a similar error of sorts. Do you reckon this issue is worth a deeper look? I could perhaps try helping if you want me to look in a particular spot. I'd ideally like this incompatibility fixed since this doesn't only affect nami adoption on our (and our clients') ecosystem side, but it may now also affect Lucid adoption - I'd like to prevent that.

Perhaps I should start by looking at the tx output serialization. Though the error in different wallets (and cardano submit api) indicate it's an issue in the signatories for some reason.

I think what other wallets do is check if the output contains an inline datum or a refScript. If so use the new output format otherwise the legacy format. I can do that too, however I don't like it actually that I'm forced to serialise transactions in a certain way although the specifications do allow me differently. Maybe wallets should sign the raw body instead of serialising it on their own and then sign it which results in wrong witnesses obviously. But I can support the legacy format for now as well.

[TotallyNotChase]

However I don't like it actually that I'm forced to serialise transactions in a certain way although the specifications do allow me differently

Completely understandable. Sometimes the ecosystem just chooses the fundamentally wrong thing..... and we devs have to just follow by also doing the wrong thing. In this particular case though, I'd argue the specification shouldn't leave room for ambiguity, and yet it does.

Maybe wallets should sign the raw body instead of serialising it on their own and then sign it which results in wrong witnesses obviously.

I always asked myself why this isn't what all the wallets do. Just seems a lot more cryptographically reasonable to me.

[alessandrokonrad]

@TotallyNotChase I just updated the vasil branch for Nami. It should support now the legacy output serialization. Actually Nami makes now the witness on the raw cbor encoded tx body that is provided. So it should work for any tx.

And you may wanna check out Lucid 0.5.4 which also has the legacy output support now. Lemme know how all of this goes.

[TotallyNotChase]

It does indeed seem to have been fixed. Amazing work!

[TotallyNotChase]

Did this change? I was told by someone about this issue popping up again in the newest nami build.

[alessandrokonrad]

Did this change? I was told by someone about this issue popping up again in the newest nami build.

Mhh lemme check. Should not be the case tho. I make sure now the tx body's original bytes are saved when deserializing. And when serializing again simply the bytes are used.

[alessandrokonrad]

@TotallyNotChase can you send me an example tx in cbor where it's failing for you? I can't figure out the "issue". The body is signed with the original bytes.

[TotallyNotChase]

Must be a false alarm then. I went to re-check myself and everything seems fine. I'll check with the person who reported the issue and see if it's something wrong on their own code side.

Sorry for the necro!

[wutzebaer]

Hi @alessandrokonrad,

i stll get this error from different customers since our transactions are built with babbage protocol parameters.

"cborHex": "84a500818258203b2af62c3ddcabd9ac3a96448970cf51da20105eda592ef7461eb6f0e703a67700018282581d617275c3ef55fddaf6c1c090fb5b2ba47a7c77298ceb790c231b8263b81a055a8fc3825839014ccabec5d245967d24f8f6f2e7ddc0fa572b94c6344575c12dd0e7f852f2f8895a296ab5e5efd3b0de3782fe52a6ee040e284d40c3324a3c1a001e8480021a0002babd031a0450efc40758206472c33f430da3aab16b3a46918cecea72af7f2400f8d76f534e41b166ee0b80a10081825820b58999a53da37627d0bbc3e44adf4b6293cb0061100e4f21b111d103b53e416f5840c9292b229bc473a3ca6e216b824f9f47a6115d20aee2c08c24cdc8682e04d445a408d7931a58ea4b031ca543d1edd043b573ba668ea9e65a986fb36dc981f703f5d90103a100a11922b9a17173656c656374656453706c696e7465727380"

Command failed: transaction submit Error: Error while submitting tx: ShelleyTxValidationError ShelleyBasedEraBabbage (ApplyTxError [UtxowFailure (FromAlonzoUtxowFail (WrappedShelleyEraFailure (InvalidWitnessesUTXOW [VKey (VerKeyEd25519DSIGN "b58999a53da37627d0bbc3e44adf4b6293cb0061100e4f21b111d103b53e416f")])))])

Using Nami 3.3.0

[waalge]

Maybe wallets should sign the raw body instead of serialising it on their own and then sign it which results in wrong witnesses obviously.

I always asked myself why this isn't what all the wallets do. Just seems a lot more cryptographically reasonable to me.

I would dearly love for this to be what CIP 30 says. Ie: Sign the data you are given, not your own version of events . Or, at the very least : Return the signature AND the serialized form of the tx (body?) The latter breaks for multisig.

Would it be possible for the CIP to be changed to this?