Skip to content
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

Added deserialisation tests for length-of Plutus ByteStrings > 64. #2216

Merged
merged 1 commit into from
Apr 20, 2021

Conversation

TimSheard
Copy link
Contributor

In order to validate data in both AuxiliaryData and in the WitnessSet, deserializing
a Plutus Data, with a ByteString type, whose length is greater than 64 now raises
a deserialisation error. Added a new combinator (<?) to Data.Coders that lets any
'constructor' raise an error. Also added roundtrip tests to be sure we actually
catch these cases.

Copy link
Contributor

@JaredCorduan JaredCorduan left a comment

Choose a reason for hiding this comment

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

If we can add the FromCBOR (Annotator (Data era)) back in, I think it looks great!

@TimSheard TimSheard force-pushed the ts-checkPlutusBytes-toolong branch from df31ed1 to 2890d85 Compare April 9, 2021 18:57
Copy link
Contributor

@nc6 nc6 left a comment

Choose a reason for hiding this comment

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

Shouldn't add additional consttaints on the API types, otherwise looks fine.

@TimSheard
Copy link
Contributor Author

I cleaned this up, addressed the (Show(CompactAddr era)) comments from Nick, rebased on the most recent master.
I am still getting a serialisation error in the benchmarks.

bench: Failed to decode state: DecoderErrorDeserialiseFailure "state" (DeserialiseFailure 6689 "expected integer")
CallStack (from HasCallStack):
error, called at bench/Bench/Cardano/Ledger/ApplyTx.hs:100:25 in main:Bench.Cardano.Ledger.ApplyTx
Benchmark bench: ERROR
cabal: Benchmarks failed for bench:bench from cardano-ledger-test-0.1.0.0.

@TimSheard TimSheard force-pushed the ts-checkPlutusBytes-toolong branch 3 times, most recently from 9e9a627 to d85d952 Compare April 19, 2021 20:16
In order to validate data in both AuxiliaryData and in the WitnessSet, deserializing
a Plutus Data, with a ByteString type, whose length is greater than 64 now raises
a deserialisation error. Added a new combinator (<?) to Data.Coders that lets any
'constructor' raise an error. Also added roundtrip tests to be sure we actually
catch these cases.

Before running plutus scripts, we collect their input data using their hashes and the Witnesses.
Previous PredicateFailure tests should ensure we find Data for every script, BUT
the consequences of not finding Data means scripts can get dropped, so things
might validate that shouldn't. So we double check that every Script has its Data, and
if that is not the case, a PredicateFailure is raised in the Utxos rule.
@nc6 nc6 merged commit acf0c4f into master Apr 20, 2021
@iohk-bors iohk-bors bot deleted the ts-checkPlutusBytes-toolong branch April 20, 2021 09:15
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