Open JaredCorduan opened 2 years ago
Is it possible to share the transaction which caused this?
@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.
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
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?
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).
I see, do you mean add instead of remove?
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?
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.
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?
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).
That makes sense - thanks!
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 dd13fada
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.
Investigate the following alonzo test failure