Reintroduce and fix stringAtreeValue #1174
Conversation
| value, err := storable.StoredValue(interpreter.Storage) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| if _, ok := value.(stringAtreeValue); ok { | ||
| equal, err := stringAtreeComparator(interpreter.Storage, value, otherStorable) |
There was a problem hiding this comment.
Both the LHS storable and the RHS storable (inside stringAtreeComparator) are now correctly loaded as values, handling the case where either of them is a storage ID storable
| } | ||
|
|
||
| func stringAtreeComparator(storage atree.SlabStorage, value atree.Value, otherStorable atree.Storable) (bool, error) { | ||
| otherValue, err := otherStorable.StoredValue(storage) |
There was a problem hiding this comment.
We correctly load the value for the other storable here and do not incorrectly cast it directly to stringAtreeValue
| key := MustConvertStoredValue(atreeKey) | ||
| keyCopy := interpreter.CopyValue(getLocationRange, key, address) | ||
| // NOTE: key is stringAtreeValue | ||
| // and does not need to be converted or copied |
There was a problem hiding this comment.
I'm not sure, but do we need to copy key here? Just double-checking. 😄
There was a problem hiding this comment.
The key is not a Cadence interpreter.Value, so there is nothing to deep copy
There was a problem hiding this comment.
Also, if it would be, copying it is literally just using it, as it is effectively a Go string (value type, immutable)
Reverts and fixes 17eec97, i.e. reintroduce
stringAtreeValueas theatree.Valueused for composite value field names (atree ordered map keys).This restores the behaviour, so we do not need a migration for Testnet, and the PR also fixes the problem identified by @fxamacker earlier that the old
stringAtreeValuecode was not handling the case properly where the value's size exceeds the max inline limit, and storables where casted tostringAtreeValue, but may beStorageIDStorablein that case