-
Notifications
You must be signed in to change notification settings - Fork 155
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
update calculations of the size of value and UTxO for minUTxOValue calculation #2101
Conversation
my free-range non-nix ormolu does something different : ( |
97ce633
to
35db058
Compare
I just smashed the three commits into one commit and ran ormolu |
ad1d7b5
to
502ede6
Compare
D) a blob of policyIDs | ||
E) a blob of asset names | ||
-} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
write units everywhere
Size in the heap of values, in words (to get the size in bytes multiply by 4 on a 32-bit machine or 8 on a 64-bit machine)
Word - 8 bytes
ignoring 32 bit machines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ascii art of layout
decide on linking comments to/from the compact value definition
A) a sequence of Word64s representing quantities | ||
B) a sequence of Word16s representing policyId indices | ||
C) Word16s representing asset name indices | ||
(as a special case for empty asset names, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also a sequence
wordLength * quanSize * totalNoAssets | ||
+ 2 * totalNoAssets * (index * wordLength) | ||
+ pidLength * noPIDs | ||
+ assetNamesLength |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
round up to Words
|
||
size (Value _ v) | ||
-- based on size in words stored in the compact representation of Value | ||
| v == mempty = fromIntegral $ adaWords * wordLength |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just 1
adaWords = 2 | ||
|
||
-- number of words used to represent quantity | ||
quanSize :: Int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be in bytes
8
it takes a Word64 to represent an ada amount (unpacked in the compact representation) | ||
|
||
2. CompactValueMultiAsset (used otherwise) takes an ada amount and token bundle data | ||
i) Word64 (ada) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there extra overhead here for i) and ii)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no overhead
|
||
2. CompactValueMultiAsset (used otherwise) takes an ada amount and token bundle data | ||
i) Word64 (ada) | ||
ii) Word (number of distinct types of multi-assets in the bundle) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Word type - made concrete in Alex's PR
where | ||
-- add addrHashLen for each Policy ID | ||
accum u ans = foldr accumIns (ans + policyIdLen) u | ||
repSize = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove wordLength, convert to bytes (but round up to nearest integral number of word64s)
add comment that this is in bytes
|
||
-- length of PID in bytes | ||
pidLength :: Int | ||
pidLength = 28 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use actual crypto scheme
get the length of the value level representation of hash for the given crypto scheme
-- | The size in bytes of the output of 'digest'
sizeHash :: forall h proxy. HashAlgorithm h => proxy h -> Word
sizeHash _ = fromInteger (natVal (Proxy @(SizeHash h)))
|
||
-- number of words used to store number of MAs in a value | ||
noMAs :: Int | ||
noMAs = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use this instead of calculated number of MAs
@@ -130,45 +131,30 @@ scaledMinDeposit v (Coin mv) | |||
-- The calculation should represent this equation | |||
-- minValueParameter / coinUTxOSize = actualMinValue / valueUTxOSize | |||
-- actualMinValue = (minValueParameter / coinUTxOSize) * valueUTxOSize | |||
| otherwise = Coin $ adaPerUTxOByte * (utxoEntrySizeWithoutVal + size v) -- round down | |||
| otherwise = Coin $ adaPerUTxOByte * (utxoEntrySizeWithoutVal + size v) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
impose mv to be a lower bound
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this calc to UTxO rule in Mary?
addrHashLen = 28 | ||
-- lengths obtained from tracing on HeapWords of inputs and outputs, plus 6 for Map overhead | ||
txoutLen :: Integer | ||
txoutLen = 14 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should depend on crypto (same as above comment)
exclude byron addresses from getting MAs
hashLen = 32 | ||
-- unpacked CompactCoin Word64 size in words | ||
coinSize :: Integer | ||
coinSize = fromIntegral $ heapWordsUnpacked (Coin 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this to compact coin
utxoEntrySizeWithoutVal :: Integer | ||
utxoEntrySizeWithoutVal = inputSize + outputSizeWithoutVal | ||
utxoEntrySizeWithoutVal = (6 + txoutLen + txinLen) * wordSize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6 overhead in heapwords for each map entry
adaPerUTxOByte :: Integer | ||
adaPerUTxOByte = quot mv (utxoEntrySizeWithoutVal + uint) | ||
adaPerUTxOByte = quot mv (utxoEntrySizeWithoutVal + coinSize * wordSize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better comment
502ede6
to
95bc3e3
Compare
8dcb419
to
4fe819b
Compare
replaced by #2107 |
HeapWords
instances and some estimates of size, this PR gives a more accurate estimate of the size of a single UTxO entry in bytes (for these estimates, see https://github.com/input-output-hk/cardano-ledger-specs/compare/polina/utxosize - the log of the tests should contain the traces of theTxOut
andTxIn
HeapWords)size
calculation inValue.hs
now estimates the size of a Value according to the new CompactValue representation