search

IntersectMBO / formal-ledger-specifications

Formal specifications of the cardano ledger
Apache License 2.0
36 stars 13 forks source link

Reference script limitations are incomplete #537

Open WhatisRT opened 1 month ago

WhatisRT commented 1 month ago

We need to add an upper bound on the reference scripts allowed per transaction and per block. Additionally, the cost was changed in the implementation to a non-linear formula (see getConwayMinFeeTx in https://github.com/IntersectMBO/cardano-ledger/blob/master/eras/conway/impl/src/Cardano/Ledger/Conway/Tx.hs).

jmchapman commented 3 weeks ago

Do we have a size estimate for this?

WhatisRT commented 3 weeks ago

This is pretty small, should be doable in 1-2 days.

williamdemeo commented 3 weeks ago

@jmchapman @WhatisRT This would be much smaller/easier task if the description were less vague. We discussed the issue briefly in the last ledger meeting, but we didn't specify, for example, what should the upper bound be?

Also, it would be very helpful to me if more things about the issue were made explicit, like which files (functions, types, etc) will likely need modifying.

Re. "an upper bound on the reference scripts allowed per transaction and per block," does this mean an upper bound on the number of reference scripts, or the size of a reference script? (This may end up being a stupid question, but based on past experience, it's important that I clear up any possible misunderstandings in advance.)

To which module will this bound likely be added?

Re. "the cost was changed in the implementation to a non-linear formula (see getConwayMinFeeTx in https://github.com/IntersectMBO/cardano-ledger/blob/master/eras/conway/impl/src/Cardano/Ledger/Conway/Tx.hs)" I believe in the meeting you indicated that we need to implement the new cost formula and probably add the formula to the appendix of the pdf. Where is the current cost function? Where should this change go?

After some digging around I could probably guess, with a relatively high degree of accuracy, the answers to some of these questions, but I suspect the answers are obvious to you and by including them in the issue description you would save me a lot of time and ensure that I can get the task done withing the time frame you suggest above.

WhatisRT commented 2 weeks ago

The per transaction limit should be an upper bound on ∑[ x ← mapValues scriptSize (setToHashMap (refScripts tx utxo)) ] x (which is currently used in minfee and probably should become a function on its own). It could be part of UTXO or UTXOW, either is fine. If the implementation does it in either of those, it's probably a good idea to match that. The upper bound should be a new protocol parameter.

For the block limit, just add all of the individual sizes per transaction and compare that with a new parameter. We don't have a BBODY STS yet, so it should go into CHAIN.

The current cost is just the above term that's part of minfee. To be able to put this new function in an appendix, it'd be easiest to just make a new module that just contains this function together with an explanation.

williamdemeo commented 2 weeks ago

It would be helpful to include the following link in the issue description for context and a detailed explanation of the calculations involved:

https://github.com/IntersectMBO/cardano-ledger/blob/master/docs/adr/2024-08-14_009-refscripts-fee-change.md