Alonzo test failure

[JaredCorduan]

Investigate the following alonzo test failure

$ cabal test cardano-ledger-alonzo-test --test-options='--quickcheck-replay=535047 -p "Fast Alonzo Property Tests"'

...
Test suite cardano-ledger-alonzo-test: RUNNING...

Running in scenario: Development
Alonzo tests
  Fast Alonzo Property Tests
    total amount of Ada is preserved (Chain): 
Discarded trace: MinFeee violation: genEraDne: AlonzoEraGen.hs

Discarded trace: TotExUnits violation: genEraTweakBlock: AlonzoEraGen.hs

Discarded trace: No inputs left. Utxo.hs
FAIL (21.81s)
      *** Failed! (after 15 tests):
      Exception while generating shrink-list:
        LEDGER sigGen: [UtxowFailure (WrappedShelleyEraFailure (ExtraneousScriptWitnessesUTXOW (fromList [ScriptHash "42c7a014a4cd5537f64e5ae8ec7349db3d8603e16765dc37f8fb6e67"])))]
        CallStack (from HasCallStack):
          error, called at src/Test/Cardano/Ledger/Shelley/Generator/Trace/Ledger.hs:179:24 in cardano-ledger-shelley-test-0.1.0.0-inplace:Test.Cardano.Ledger.Shelley.Generator.Trace.Ledger
          genAndApplyTx, called at src/Test/Cardano/Ledger/Shelley/Generator/Trace/Ledger.hs:159:11 in cardano-ledger-shelley-test-0.1.0.0-inplace:Test.Cardano.Ledger.Shelley.Generator.Trace.Ledger
          sigGen, called at src/Test/Cardano/Ledger/Shelley/Generator/Block.hs:101:16 in cardano-ledger-shelley-test-0.1.0.0-inplace:Test.Cardano.Ledger.Shelley.Generator.Block
      Use --quickcheck-replay=535047 to reproduce.

1 out of 1 tests failed (21.82s)
Test suite cardano-ledger-alonzo-test: FAIL

[jmhrpr]

Is it possible to share the transaction which caused this?

[JaredCorduan]

@jmhrpr it would take a bit of digging, but I'm sure we could if it was important. It looks like the generators are just adding an unneeded script witness, though.

[jmhrpr]

I guess it was just concerning that this could hint that in some case the ledger is not requiring a script when it be, but thinking about it that would probably throw other errors for extra redeemers etc so yeah probably like you say.

Though, now I'm looking at the functionality:
https://github.com/input-output-hk/cardano-ledger/blob/16e4498ab762b77eca4a6191a3d7845a869c13af/eras/babbage/impl/src/Cardano/Ledger/Babbage/Rules/Utxow.hs#L185-L189

https://github.com/input-output-hk/cardano-ledger/blob/16e4498ab762b77eca4a6191a3d7845a869c13af/eras/babbage/impl/src/Cardano/Ledger/Babbage/Rules/Utxow.hs#L86-L94

If you have the same required script s1 inlined in a spend or reference input as well as in the witness set script map, then sRefs = {s1}, sReceived = {s1} and sNeeded = {s1}.

On L92 we remove sRefs from sNeeded to get neededNonRefs = {}.

On L94 we then check the difference of sReceived = {s1} and neededNonRefs = {}, and get the ExtraneousScriptWitnessesUTXOW failure because the difference is s1 instead of the empty set.

That doesn't feel like the correct behaviour, if I'm not misunderstanding. What do you think - shall I raise an issue?

[WhatisRT]

This is intended behaviour, if you check the spec that it will also fail in this situation. The reason is that we want to ensure that the block producer can not remove any scripts from the transaction (which is a property that Alonzo has, so we want to preserve it).

[jmhrpr]

I see, do you mean add instead of remove?

[jmhrpr]

If a Tx requires s1 and it is included inlined in an input (sRefs = {s1}), and also in the witnesses (sReceived = {s1}) then neededNonRefs = sNeeded - sRefs = {}. Regardless of whether the BP removes s1 from the witnesses of the transaction we have missing = {} on L93 because neededNonRefs = {}. We have extra = {} on L94 if the BP removed s1 from the witnesses because that will mean sReceived = {}, so no errors if the BP removes s1 from the witnesses when s1 also inlined. I think?

[WhatisRT]

Adding as well as removing scripts should be impossible, i.e. we want the set of scripts to be specified exactly. If we allowed scripts to be both present in the witnesses and be inlined, that wouldn't be the case.

[jmhrpr]

Ah, yes mistake in the scenario described in my last message is that it has the script both inlined and in the witnesses, but we're saying we don't want to allow that for the very reason I showed - if the ledger rules did allow a script to be both inlined and in the witnesses then:

1. if a TX had a script inlined and in the witnesses a third-party can remove the script from the witnesses without an error;
2. if a TX had a script which was only inlined and not in the witness set then a third party could add that script to the witness set without an error.

Am I understanding correctly? Though I'm not sure what we are protecting against (if anything) - I can't immediately see any issues that would arise from 1), and increasing the transaction size via 2) would at worse cause phase1 failure similar to changing anything else in the transaction. If either case could affect phase2 script execution then that would be an issue but I don't think they can. I guess perhaps we just want to provide users with the guarantee that the transactions they submit will be identical to those that appear on chain?

[WhatisRT]

Yes, that's what it is about. It's not really that we think there's an attack, or that it's particularly bad if something changed that doesn't affect anything, it's just that this is a nice property to have: no one gets to mess with your transactions, whether that actually affects anything or not. Note that this is sadly already broken, but only it the VKey witnesses (think of multisig: if you build a transaction, you might not know who is going to sign it, but you don't really care as long as enough people do).

[jmhrpr]

That makes sense - thanks!

[Soupstraw]

That seed does not seem to work on the current master branch, but I ran a bunch of tests to try to reproduce it and managed to make the same test fail with missing scripts this time:

Build profile: -w ghc-8.10.7 -O1
In order, the following will be built (use -v for more details):
 - cardano-ledger-alonzo-test-0.1.0.0 (test:cardano-ledger-alonzo-test) (first run)
Preprocessing test suite 'cardano-ledger-alonzo-test' for cardano-ledger-alonzo-test-0.1.0.0..
Building test suite 'cardano-ledger-alonzo-test' for cardano-ledger-alonzo-test-0.1.0.0..
Running 1 test suites...
Test suite cardano-ledger-alonzo-test: RUNNING...

Running in scenario: Development
Alonzo tests
  Fast Alonzo Property Tests
    total amount of Ada is preserved (Chain): 
Discarded trace: TotExUnits violation: genEraTweakBlock: AlonzoEraGen.hs

Discarded trace: MinFeee violation: genEraDne: AlonzoEraGen.hs

Discarded trace: No inputs left. Utxo.hs

Discarded trace: NoMoneyleft Utxo.hs
FAIL (74.43s)
      *** Failed! (after 36 tests):
      Exception while generating shrink-list:
        LEDGER sigGen: [UtxowFailure (WrappedShelleyEraFailure (MissingScriptWitnessesUTXOW (fromList [])))]
        CallStack (from HasCallStack):
          error, called at src/Test/Cardano/Ledger/Shelley/Generator/Trace/Ledger.hs:179:24 in cardano-ledger-shelley-test-0.1.0.0-inplace:Test.Cardano.Ledger.Shelley.Generator.Trace.Ledger
          genAndApplyTx, called at src/Test/Cardano/Ledger/Shelley/Generator/Trace/Ledger.hs:159:11 in cardano-ledger-shelley-test-0.1.0.0-inplace:Test.Cardano.Ledger.Shelley.Generator.Trace.Ledger
          sigGen, called at src/Test/Cardano/Ledger/Shelley/Generator/Block.hs:101:16 in cardano-ledger-shelley-test-0.1.0.0-inplace:Test.Cardano.Ledger.Shelley.Generator.Block
      Use --quickcheck-replay=970901 to reproduce.

1 out of 1 tests failed (74.46s)
Test suite cardano-ledger-alonzo-test: FAIL
Test suite logged to:
/home/work/Projects/cardano-ledger/dist-newstyle/build/x86_64-linux/ghc-8.10.7/cardano-ledger-alonzo-test-0.1.0.0/t/cardano-ledger-alonzo-test/test/cardano-ledger-alonzo-test-0.1.0.0-cardano-ledger-alonzo-test.log
0 of 1 test suites (0 of 1 test cases) passed.

commit dd13fad

[JaredCorduan]

Another example found here: https://github.com/input-output-hk/cardano-ledger/runs/7131338110?check_suite_focus=true

Running in scenario: Development
Alonzo tests
  Fast Alonzo Property Tests
    total amount of Ada is preserved (Chain):                       OK (6.31s)
        ✓ Relevant UBLOCK traces are covered passed 300 tests.
          at least 20% of the proposals get confirmed                95% ███████████████████· ✓ 75%
          at least 30% of the proposals get unconfirmed              90% █████████████████▉·· ✓ 75%
          at least 2% of the proposals get voted in the same block   98% ███████████████████▋ ✓ 75%
          at least 30% of blocks contain no votes                   100% ████████████████████ ✓ 75%
          at least 10% of blocks contain votes                      100% ████████████████████ ✓ 70%
          at least 70% of the blocks contain no update proposals    100% ████████████████████ ✓ 75%
          at least 10% of the blocks contain an update proposals     99% ███████████████████▋ ✓ 75%
          at least 10% of the proposals get enough endorsements      93% ██████████████████▌· ✓ 60%
          at least 5% of the proposals get enough endorsements       95% ███████████████████· ✓ 80%
    Invalid registrations are generated when requested: FAIL (6.33s)
      *** Failed! (after 2 tests):
      Exception while generating shrink-list:
        LEDGER sigGen: [UtxowFailure (WrappedShelleyEraFailure (ExtraneousScriptWitnessesUTXOW (fromList [ScriptHash "42c7a014a4cd5537f64e5ae8ec7349db3d8603e16765dc37f8fb6e67"])))]
        CallStack (from HasCallStack):
          error, called at src/Test/Cardano/Ledger/Shelley/Generator/Trace/Ledger.hs:179:24 in cardano-ledger-shelley-test-0.1.0.0-inplace:Test.Cardano.Ledger.Shelley.Generator.Trace.Ledger
          genAndApplyTx, called at src/Test/Cardano/Ledger/Shelley/Generator/Trace/Ledger.hs:159:11 in cardano-ledger-shelley-test-0.1.0.0-inplace:Test.Cardano.Ledger.Shelley.Generator.Trace.Ledger
          sigGen, called at src/Test/Cardano/Ledger/Shelley/Generator/Block.hs:101:16 in cardano-ledger-shelley-test-0.1.0.0-inplace:Test.Cardano.Ledger.Shelley.Generator.Block
      Use --quickcheck-replay=236891 to reproduce.