NethermindEth / juno

Starknet client implementation.
https://juno.nethermind.io
Apache License 2.0
381 stars 164 forks source link

Truncate function issue #1966

Open AnkushinDaniil opened 1 month ago

AnkushinDaniil commented 1 month ago

Truncate function There is a commit with a fuzz test and a possible solution. Input:

k.len = 7
k.bitset = [32]uint8 [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,64]
length = 2

output:

k.len = 2
k.bitset = [32]uint8 [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]

Shouldn't the length be 0?

kirugan commented 1 month ago

Sorry I thought this is a PR. Cleared assignees

obasekiosa commented 3 weeks ago

@AnkushinDaniil

Could you explain some more. I don't understand the question/statement?

AnkushinDaniil commented 3 weeks ago

@obasekiosa

64 = 0b1000000 If truncate to length = 2, we have 0b00 or 0b0, and if we count significant figures, the length should be 0 or 1.

obasekiosa commented 2 weeks ago

This issue raises the concern that Truncating a key to a particular length might result in a key with a value which can be stored using a much smaller key length.

e.g a key 0b1000000 of length 7 if truncated to length 2 clears the first 5 = (7-2) bits and results in a value of 0b00. which technically is equivalent to a value of 0b0 which requires just a key length of 1 to store.

My opinion is that, the Truncate function alters the structure of the key. so a key length of 2 for the above operation is the correct output.

seeing that the current implementation uses a fixed size 256 bit array to store each key of varying length, even after truncation 0b00 or 0b0 is still stored as 0b00....0 (256 bits, all zero)

Basically a STRUCTURE vs VALUE CONTENT interpretation of key.length, does it refer to the minimum bits needed to store the key (in which case we have a bug) or does it refer to simply to the length we want the key to be in (in which case we don't have a bug and can close this issue).