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

Manually tweak e2e fixtures to be valid in Conway #4752

Merged
merged 3 commits into from
Sep 2, 2024

Conversation

Anviking
Copy link
Member

@Anviking Anviking commented Aug 26, 2024

  • Make e2e game fixtures valid in Conway by dropping two fields with empty values

Comments

When reproducing in a unit test I got the following two failures:

  • DecoderErrorDeserialiseFailure "Shelley Tx" (DeserialiseFailure 117 "TxBody: 'Required Signer Hashes' must be non-empty when supplied")
  • DecoderErrorDeserialiseFailure "Shelley Tx" (DeserialiseFailure 42 "TxBody: 'Collateral Inputs' must be non-empty when supplied")

These correspond to the tags 13 and 14 in the Conway binary spec:
https://github.com/IntersectMBO/cardano-ledger/blob/8d7d261dfb6282ab86cad32ec3f1be71db9a3080/eras/conway/impl/cddl-files/conway.cddl#L431-L432

Using cbor.me we can confirm the presence of in the binaries from the
errors:

      0D                                # unsigned(13)
      80                                # array(0)

So we manaully delete

  • 0d80
  • 0e80
    with this meaning (some matches of 0e80 were part of bytestrings, so we
    don't delete them)

And then adjust the preceding TxBody length:

84                                      # array(4)
   A5                                   # map(5)

E.g. s/A5/A3.

Issue Number

ADP-3413

@Anviking Anviking self-assigned this Aug 26, 2024
@Anviking Anviking force-pushed the anviking/ADP-3413/e2e-game-fixtures-conway branch from b1bd20e to 83b01b9 Compare August 27, 2024 00:02
@Anviking Anviking marked this pull request as ready for review August 27, 2024 11:12
@Anviking
Copy link
Member Author

Anviking commented Aug 27, 2024

It seems the custom run didn't use the changes from this PR, as we see the old binaries in the error messages. @paolino, I presume there should be a way to fix this, but we may also just try merging?

Doh, classic editing the wrong file 😅

@Anviking Anviking requested a review from paolino August 27, 2024 11:20
@Anviking Anviking force-pushed the anviking/ADP-3413/e2e-game-fixtures-conway branch 4 times, most recently from 11cc375 to 0cf78dd Compare August 30, 2024 21:28
@Anviking
Copy link
Member Author

@paolino e2e test pass (albeit with one pending). Ruby linter fails, but it doesn't look related to the one line I added. Please review and merge if you see fit.

@Anviking Anviking force-pushed the anviking/ADP-3413/e2e-game-fixtures-conway branch from 0cf78dd to ff7f9ee Compare August 31, 2024 17:05
@Anviking Anviking changed the title Manually tweak e2e game goldens to be valid in Conway Manually tweak e2e fixtures to be valid in Conway Aug 31, 2024
Copy link
Contributor

@HeinrichApfelmus HeinrichApfelmus 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! 😊

I have assuaged Rubocop. I'm going to merge this without running the optional E2E tests, as Johannes has already indicated that they are working.

Anviking and others added 3 commits September 2, 2024 16:01
When reproducing in a unit test I got the following two failures:

- DecoderErrorDeserialiseFailure "Shelley Tx" (DeserialiseFailure 117 "TxBody: 'Required Signer Hashes' must be non-empty when supplied")
- DecoderErrorDeserialiseFailure "Shelley Tx" (DeserialiseFailure 42 "TxBody: 'Collateral Inputs' must be non-empty when supplied")

These correspond to the tags 13 and 14 in the Conway binary spec:
https://github.com/IntersectMBO/cardano-ledger/blob/8d7d261dfb6282ab86cad32ec3f1be71db9a3080/eras/conway/impl/cddl-files/conway.cddl#L431-L432

Using cbor.me we can confirm the presence of in the binaries from the
errors:

```
      0D                                # unsigned(13)
      80                                # array(0)
```

So we manaully delete
- `0d80`
- `0e80`
with this meaning (some matches of 0e80 were part of bytestrings, so we
don't delete them)

And then adjust the preceding TxBody length:

```
84                                      # array(4)
   A5                                   # map(5)
```

E.g. `s/A5/A3`.
@HeinrichApfelmus HeinrichApfelmus force-pushed the anviking/ADP-3413/e2e-game-fixtures-conway branch from e90f8f2 to c24f835 Compare September 2, 2024 14:01
@HeinrichApfelmus HeinrichApfelmus merged commit 2cdeb9a into master Sep 2, 2024
20 of 26 checks passed
@HeinrichApfelmus HeinrichApfelmus deleted the anviking/ADP-3413/e2e-game-fixtures-conway branch September 2, 2024 14:07
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.

2 participants