ADR-10 Script Witness API

[Jimbo4350]

No description provided.

[carbolymer]

Nice work. My main concern is about indexing logic: #67 (comment)

[carbolymer]

L.AlonzoEraScript era

I don't see it explained in the ADR - why not use our witnesses instead? i.e. why not

WitTxIn :: AlonzoEraOnwards era => TxIn -> Witnessable TxIn era

It would be more convenient I think, without exposing ledger constraints to cardano-api users.

I'm ok with using more and more ledger types, but I'm not sure about ledger constraints. This will be more confusing for the users if they have ledger constraints in the function signature. One would have to figure out the connection between a ledger constraint and our API, meaning, find out the right function bringing the constraint into the scope.

This means they would have to deal with both: eons and ledger constraints.

In case of using our eons, we can hide that plumbing behind the scenes.

[Jimbo4350]

I added:

The ledger constraints in the Witnessable thing era data constructors allow us to directly construct the script witness index (PlutusPurpose AsIx era) with the toPlutusScriptPurpose class method. We no longer have to use the old api's ScriptWitnessIndex which is eventually converted into a PlutusPurpose AsIx era. This approach is more direct.

I'll need to think about it more. I don't see a nice way of constructing the index without adding more boiler plate or making the api more specific to either the old or experimental api.

[Jimbo4350]

WitTxIn :: AlonzoEraOnwards era => TxIn -> Witnessable TxIn era

Because this puts us back to using eons which we don't want to use. The api I created is meant to be used with the old and new api. Sometimes we do reuse types from the old api but the eons are not one of them.

With the exception of getTxScriptWitnessesRequirements but that is called in legacyWitnessToScriptRequirements which is specific to the old api.

[carbolymer]

The api I created is meant to be used with the old and new api.

But the new API will be used in mainnet era onwards, so technically, we will always have the witness right?

My point is, I don't see any harm in using eons (old api) for now, and just removing them when we're done with old api.

[carbolymer]

This is not a blocker from my side. I see exposing ledger constraints just as an inconvenience for the library user.

We can always hide them later, when moving to the new eras API, but that means one more breaking change.

[Jimbo4350]

I'll have a think about this some more and see if I can come up with a nicer solution. I also want to see how this behaves in the cli.

[carbolymer]

For certificates it's not technically 100% correct. Vide alonzo spec:

certificates in the DCert list are indexed in the order in which they arranged in the
(full, unfiltered) list of certificates inside the transaction

So if you read a transaction with an arbitrary order of certificates, and try to regenerate indices with this logic, you could get different indices.

[Jimbo4350]

Good catch, I forgot certs were in an OSet. I'll adjust the Eq instance.

[carbolymer]

This breaks Ord laws. I think it may also make some sorting algorithms not stable. Additionally, it will make the type unusable as a key in Data.Map.

[carbolymer]

I think there's no clean way to implement this using Ord. You could extend witnessable certs with their original indices e.g.:

type Cert = (Word32, AnyCertificate, StakeCredential)

data Witnessable thing era where
  ...
  WitTxCert :: L.AlonzoEraScript era => Word32 -> Cert -> Witnessable Cert era

then you could implement Ord by simply sorting on that index.

The downside is that you'll require library user to set the index explicitly. A helper function [Cert] -> [Witnessable Cert era] would help here.

[Jimbo4350]

Fixed