Transaction estimateMaxNumberOfInputs #495
Conversation
rvl
force-pushed
the
rvl/462/estimate-max-number-of-inputs
branch
2 times, most recently
from
9c08891 to
2b2f833
Compare
rvl
force-pushed
the
rvl/462/estimate-max-number-of-inputs
branch
2 times, most recently
from
38f90db to
65ce9bd
Compare
| -- There are no attributes. | ||
| , estAddressSample = | ||
| let xpub = XPub (BS.replicate 32 0) (ChainCode $ BS.replicate 32 0) | ||
| in Address $ toByteString $ encodeAddress xpub mempty |
There was a problem hiding this comment.
Note that the size of the Byron addresses is different between 'Mainnet and 'Testnet. There are actually some attributes for 'Testnet addresses (cf: keyToAddress)
This means that estimateMaxNumberOfInputsParams probably requires a type parameter t. (KeyToAddress t) => in order to correctly compute the size. The difference isn't much but, on many inputs it is significant enough to not be ignored.
This isn't a problem for Jörmungandr since the discrimination happens on a single byte, present in both mainnet and testnet!
There was a problem hiding this comment.
OK, I have added the type parameter.
| data EstimateMaxNumberOfInputsParams = EstimateMaxNumberOfInputsParams | ||
| { estMeasureTx :: [TxIn] -> [TxOut] -> [TxWitness] -> Int | ||
| -- ^ Finds the size of a serialized transaction. | ||
| , estAddressSample :: Address -- ^ Address to use in tx output |
There was a problem hiding this comment.
I wonder why not have an Int here too and only deal with an address size like you do for BlockHashSize and TxWitnessSize. In the end, both the bridge and jormungandr creates an improper address filled with null bytes, so passing a size would be less-confusing and more consistent with the rest I think ?
There was a problem hiding this comment.
I would have liked to use a plain estAddressSize :: Int but the Tx serializer wants to deserialize the addresses it is given...
There was a problem hiding this comment.
I see 🤔 .. This is only in the bridge I guess but that makes sense.
Though, the record parameter is now superfluous since we have access to t. We can construct a valid address for a target network using only a public key, and public keys always have the same size. Hence:
baseAddr = keyToAddress @t <$> (xpub $ chaff 64) There was a problem hiding this comment.
Ah yes, that simplifies things. Fixed.
| (isIncreasingFunction .&&. estIsSmallerThanSize) | ||
| where | ||
| isIncreasingFunction = if (ma < mb) then (est qa <= est qb) else (est qa >= est qb) | ||
| estIsSmallerThanSize = (est qa <= ma) .&&. (est qb <= mb) |
There was a problem hiding this comment.
This one is ... dubious 🤔 We are comparing items that have different dimensions. One is a max size in bytes, the other one a number of inputs. And here, it happens to hold because every part of a transaction is more than one byte, though I am not sure that this buys us really anything 🤔
There was a problem hiding this comment.
I can't think of many more properties. This one doesn't buy much, but it doesn't cost much either. We can't test absolute values, only the relationship between estimates with different inputs. This one is like saying, for all functions f x = x * x, then f x > x holds, or the area of a square is greater than the length of its side, if you are willing to compare different units.
rvl
force-pushed
the
rvl/462/estimate-max-number-of-inputs
branch
from
0d354d5 to
2079d29
Compare
rvl
force-pushed
the
rvl/462/estimate-max-number-of-inputs
branch
from
2079d29 to
a289889
Compare
| -> Property | ||
| propMaxNumberOfInputsEstimation tl qa@(Quantity ma) qb@(Quantity mb) oa ob = | ||
| counterexample debug | ||
| (isIncreasingFunction .&&. moreOutputsLessInputs .&&. estIsSmallerThanSize) |
There was a problem hiding this comment.
In general, I'd be more in favor of having three properties here instead of a conjunction of assertions. The error reporting from QuickCheck isn't ideal with .&&. (or conjoin), whereas, with separate properties, we immediately know which property is failing.
Relates to #462
Overview
estimateMaxNumberOfInputsfor jormungandr and http-bridge.Comments
The estimation formula is the same regardless of backend, just with different variables. But it was a hassle sharing the estimation function between backends.