Skip to content
This repository has been archived by the owner on Aug 31, 2021. It is now read-only.

VDB-318 Allow for packed storage slots #121

Merged
merged 6 commits into from
Jul 23, 2019
Merged

VDB-318 Allow for packed storage slots #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b0fff9a
First pass at decoded packed storage slots
elizabethengelman Jul 18, 2019
ebfb496
Add tests for decoding packed storage
elizabethengelman Jul 18, 2019
51915f7
Pass storage values to repo.create as a map
elizabethengelman Jul 18, 2019
d30b4ea
Update GetStorageValueMetadata
elizabethengelman Jul 19, 2019
69ad521
Apply go fmt changes and cleanup
elizabethengelman Jul 19, 2019
2c66afb
Address PR feedback
elizabethengelman Jul 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions libraries/shared/factories/storage/transformer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,54 @@ var _ = Describe("Storage transformer", func() {
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError(fakes.FakeError))
})

Describe("when a storage row contains more than one item packed in storage", func() {
var (
rawValue = common.HexToAddress("000000000000000000000000000000000000000000000002a300000000002a30")
fakeBlockNumber = 123
fakeBlockHash = "0x67890"
packedTypes = make(map[int]utils.ValueType)
)
packedTypes[0] = utils.Uint48
packedTypes[1] = utils.Uint48

var fakeMetadata = utils.StorageValueMetadata{
Name: "",
Keys: nil,
Type: utils.PackedSlot,
PackedTypes: packedTypes,
}

It("passes the decoded data items to the repository", func() {
mappings.Metadata = fakeMetadata
fakeRow := utils.StorageDiffRow{
Contract: common.Address{},
BlockHash: common.HexToHash(fakeBlockHash),
BlockHeight: fakeBlockNumber,
StorageKey: common.Hash{},
StorageValue: rawValue.Hash(),
}

err := t.Execute(fakeRow)

Expect(err).NotTo(HaveOccurred())
Expect(repository.PassedBlockNumber).To(Equal(fakeBlockNumber))
Expect(repository.PassedBlockHash).To(Equal(common.HexToHash(fakeBlockHash).Hex()))
Expect(repository.PassedMetadata).To(Equal(fakeMetadata))
expectedPassedValue := make(map[int]string)
expectedPassedValue[0] = "10800"
expectedPassedValue[1] = "172800"
Copy link

@aaizuss aaizuss Jul 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that 2a300 is "172800" in decimal and 2a30 is "10800", which is why you're setting these expectedPassedValues, but I don't understand how you know where to get the 2 slots from "000000000000000000000000000000000000000000000002a300000000002a30" for the expected values (or how the 0 padding works - is that in the solidity documentation?). Why is 2a30 the first slot and 2a300 the second? how do you know the second isn't 2a3000

edit: I commented on this before seeing the decodePackedSlot function, which looks like it might help me understand

Expect(repository.PassedValue.(map[int]string)).To(Equal(expectedPassedValue))
})

It("returns error if creating a row fails", func() {
mappings.Metadata = fakeMetadata
repository.CreateErr = fakes.FakeError

err := t.Execute(utils.StorageDiffRow{StorageValue: rawValue.Hash()})

Expect(err).To(HaveOccurred())
Expect(err).To(MatchError(fakes.FakeError))
})
})
})
67 changes: 59 additions & 8 deletions libraries/shared/storage/utils/decoder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,31 +23,82 @@ import (
"github.com/ethereum/go-ethereum/common"
)

const (
bitsPerByte = 8
)

func Decode(row StorageDiffRow, metadata StorageValueMetadata) (interface{}, error) {
switch metadata.Type {
case Uint256:
return decodeUint256(row.StorageValue.Bytes()), nil
return decodeInteger(row.StorageValue.Bytes()), nil
case Uint48:
return decodeUint48(row.StorageValue.Bytes()), nil
return decodeInteger(row.StorageValue.Bytes()), nil
case Uint128:
return decodeInteger(row.StorageValue.Bytes()), nil
case Address:
return decodeAddress(row.StorageValue.Bytes()), nil
case Bytes32:
return row.StorageValue.Hex(), nil
case PackedSlot:
return decodePackedSlot(row.StorageValue.Bytes(), metadata.PackedTypes), nil
default:
panic(fmt.Sprintf("can't decode unknown type: %d", metadata.Type))
}
}

func decodeUint256(raw []byte) string {
n := big.NewInt(0).SetBytes(raw)
return n.String()
}

func decodeUint48(raw []byte) string {
func decodeInteger(raw []byte) string {
n := big.NewInt(0).SetBytes(raw)
return n.String()
}

func decodeAddress(raw []byte) string {
return common.BytesToAddress(raw).Hex()
}

func decodePackedSlot(raw []byte, packedTypes map[int]ValueType) map[int]string {
storageSlotData := raw
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this assignment is superfluous; could name the argument storageSlotData instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assigned the raw argument to storageSlotData because then on line 84 we're updating what is in storageSlotData by popping off the current item. Does that make sense?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah that makes perfect sense, sorry I missed that!

decodedStorageSlotItems := map[int]string{}
numberOfTypes := len(packedTypes)

for position := 0; position < numberOfTypes; position++ {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could maybe replace this with for k, v := range packedTypes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maps also don't have a guaranteed iteration order (golang actually purposefully randomizes the intersection order 😆: https://blog.golang.org/go-maps-in-action), so if we iterate over them with for k, v := range packedTypes they won't necessarily decode the raw data in the right order.

I think an alternative to the current implementation could be to sort the packedTypes keys, and then do something like this

var keys []int
for k := range packedTypes {
    keys = append(keys, k)
}
sort.Ints(keys)
for _, k := range keys {
   // similar loop
}

Do you think that's more readable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I'm fine to stick with the current implementation given that we need a guaranteed iteration order.

I am wondering how much it would muck things up to try an implementation that doesn't require a specific iteration order. Thinking that perhaps we could implement a function that returns the relevant slice for a given index provided the indexes and types of all values packed into the slot, but I'm also not sure whether or not that would be more readable 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, i need to think about this a bit more. 🤔 do you think it's worth adding a story in the back log to address this later?

//get length of remaining storage date
lengthOfStorageData := len(storageSlotData)

//get item details (type, length, starting index, value bytes)
itemType := packedTypes[position]
lengthOfItem := getNumberOfBytes(itemType)
itemStartingIndex := lengthOfStorageData - lengthOfItem
itemValueBytes := storageSlotData[itemStartingIndex:]

//decode item's bytes and set in results map
decodedValue := decodeIndividualItems(itemValueBytes, itemType)
decodedStorageSlotItems[position] = decodedValue

//pop last item off raw slot data before moving on
storageSlotData = storageSlotData[0:itemStartingIndex]
}

return decodedStorageSlotItems
}

func decodeIndividualItems(itemBytes []byte, valueType ValueType) string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this should be singular?

switch valueType {
case Uint48:
return decodeInteger(itemBytes)
case Uint128:
return decodeInteger(itemBytes)
default:
panic(fmt.Sprintf("can't decode unknown type: %d", valueType))
}
}

func getNumberOfBytes(valueType ValueType) int {
switch valueType {
case Uint48:
return 48 / bitsPerByte
case Uint128:
return 128 / bitsPerByte
default:
panic(fmt.Sprintf("ValueType %d not recognized", valueType))
}
}
88 changes: 88 additions & 0 deletions libraries/shared/storage/utils/decoder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,17 @@ var _ = Describe("Storage decoder", func() {
Expect(result).To(Equal(big.NewInt(0).SetBytes(fakeInt.Bytes()).String()))
})

It("decodes uint128", func() {
fakeInt := common.HexToHash("0000000000000000000000000000000000000000000000000000000000011123")
row := utils.StorageDiffRow{StorageValue: fakeInt}
metadata := utils.StorageValueMetadata{Type: utils.Uint128}

result, err := utils.Decode(row, metadata)

Expect(err).NotTo(HaveOccurred())
Expect(result).To(Equal(big.NewInt(0).SetBytes(fakeInt.Bytes()).String()))
})

It("decodes uint48", func() {
fakeInt := common.HexToHash("0000000000000000000000000000000000000000000000000000000000000123")
row := utils.StorageDiffRow{StorageValue: fakeInt}
Expand All @@ -59,4 +70,81 @@ var _ = Describe("Storage decoder", func() {
Expect(err).NotTo(HaveOccurred())
Expect(result).To(Equal(fakeAddress.Hex()))
})

Describe("when there are multiple items packed in the storage slot", func() {
It("decodes uint48 items", func() {
//this is a real storage data example
packedStorage := common.HexToHash("000000000000000000000000000000000000000000000002a300000000002a30")
row := utils.StorageDiffRow{StorageValue: packedStorage}
packedTypes := map[int]utils.ValueType{}
packedTypes[0] = utils.Uint48
packedTypes[1] = utils.Uint48

metadata := utils.StorageValueMetadata{
Type: utils.PackedSlot,
PackedTypes: packedTypes,
}

result, err := utils.Decode(row, metadata)
decodedValues := result.(map[int]string)

Expect(err).NotTo(HaveOccurred())
Expect(decodedValues[0]).To(Equal(big.NewInt(0).SetBytes(common.HexToHash("2a30").Bytes()).String()))
Expect(decodedValues[1]).To(Equal(big.NewInt(0).SetBytes(common.HexToHash("2a300").Bytes()).String()))
})

It("decodes 5 uint48 items", func() {
//TODO: this packedStorageHex was generated by hand, it would be nice to test this against
//real storage data that has several items packed into it
packedStorageHex := "0000000A5D1AFFFFFFFFFFFE00000009F3C600000002A300000000002A30"

packedStorage := common.HexToHash(packedStorageHex)
row := utils.StorageDiffRow{StorageValue: packedStorage}
packedTypes := map[int]utils.ValueType{}
packedTypes[0] = utils.Uint48
packedTypes[1] = utils.Uint48
packedTypes[2] = utils.Uint48
packedTypes[3] = utils.Uint48
packedTypes[4] = utils.Uint48

metadata := utils.StorageValueMetadata{
Type: utils.PackedSlot,
PackedTypes: packedTypes,
}

result, err := utils.Decode(row, metadata)
decodedValues := result.(map[int]string)

Expect(err).NotTo(HaveOccurred())
Expect(decodedValues[0]).To(Equal(big.NewInt(0).SetBytes(common.HexToHash("2a30").Bytes()).String()))
Expect(decodedValues[1]).To(Equal(big.NewInt(0).SetBytes(common.HexToHash("2a300").Bytes()).String()))
Expect(decodedValues[2]).To(Equal(big.NewInt(0).SetBytes(common.HexToHash("9F3C6").Bytes()).String()))
Expect(decodedValues[3]).To(Equal(big.NewInt(0).SetBytes(common.HexToHash("FFFFFFFFFFFE").Bytes()).String()))
Expect(decodedValues[4]).To(Equal(big.NewInt(0).SetBytes(common.HexToHash("A5D1A").Bytes()).String()))
})

It("decodes 2 uint128 items", func() {
//TODO: this packedStorageHex was generated by hand, it would be nice to test this against
//real storage data that has several items packed into it
packedStorageHex := "000000038D7EA4C67FF8E502B6730000" +
"0000000000000000AB54A98CEB1F0AD2"
packedStorage := common.HexToHash(packedStorageHex)
row := utils.StorageDiffRow{StorageValue: packedStorage}
packedTypes := map[int]utils.ValueType{}
packedTypes[0] = utils.Uint128
packedTypes[1] = utils.Uint128

metadata := utils.StorageValueMetadata{
Type: utils.PackedSlot,
PackedTypes: packedTypes,
}

result, err := utils.Decode(row, metadata)
decodedValues := result.(map[int]string)

Expect(err).NotTo(HaveOccurred())
Expect(decodedValues[0]).To(Equal(big.NewInt(0).SetBytes(common.HexToHash("AB54A98CEB1F0AD2").Bytes()).String()))
Expect(decodedValues[1]).To(Equal(big.NewInt(0).SetBytes(common.HexToHash("38D7EA4C67FF8E502B6730000").Bytes()).String()))
})
})
})
41 changes: 34 additions & 7 deletions libraries/shared/storage/utils/value.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,54 @@

package utils

import "fmt"

type ValueType int

const (
Uint256 ValueType = iota
Uint48
Uint128
Bytes32
Address
PackedSlot
)

type Key string

type StorageValueMetadata struct {
Name string
Keys map[Key]string
Type ValueType
Name string
Keys map[Key]string
Type ValueType
PackedNames map[int]string //zero indexed position in map => name of packed item
PackedTypes map[int]ValueType //zero indexed position in map => type of packed item
}

func GetStorageValueMetadata(name string, keys map[Key]string, valueType ValueType) StorageValueMetadata {
return getMetadata(name, keys, valueType, nil, nil)
}

func GetStorageValueMetadataForPackedSlot(name string, keys map[Key]string, valueType ValueType, packedNames map[int]string, packedTypes map[int]ValueType) StorageValueMetadata {
return getMetadata(name, keys, valueType, packedNames, packedTypes)
}

func GetStorageValueMetadata(name string, keys map[Key]string, t ValueType) StorageValueMetadata {
func getMetadata(name string, keys map[Key]string, valueType ValueType, packedNames map[int]string, packedTypes map[int]ValueType) StorageValueMetadata {
assertPackedSlotArgs(valueType, packedNames, packedTypes)

return StorageValueMetadata{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably unnecessary but I'm wondering if we might want to put a guard here to check that if t == PackedSlot then packedNames != nil and packedTypes != nil (and vice versa if t != PackedSlot)

Name: name,
Keys: keys,
Type: t,
Name: name,
Keys: keys,
Type: valueType,
PackedNames: packedNames,
PackedTypes: packedTypes,
}
}

func assertPackedSlotArgs(valueType ValueType, packedNames map[int]string, packedTypes map[int]ValueType) {
if valueType == PackedSlot && (packedTypes == nil || packedNames == nil) {
panic(fmt.Sprintf("ValueType is PackedSlot. Expected PackedNames and PackedTypes to not be nil, but got PackedNames = %v and PackedTypes = %v", packedNames, packedTypes))
} else if (packedNames != nil && packedTypes != nil) && valueType != PackedSlot {
panic(fmt.Sprintf("PackedNames and PackedTypes passed in. Expected ValueType to equal PackedSlot (%v), but got %v.", PackedSlot, valueType))
}

}
58 changes: 57 additions & 1 deletion libraries/shared/storage/utils/value_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
)

var _ = Describe("Storage value metadata getter", func() {
It("returns a storage value metadata instance with corresponding fields assigned", func() {
It("returns storage value metadata for a single storage variable", func() {
metadataName := "fake_name"
metadataKeys := map[utils.Key]string{"key": "value"}
metadataType := utils.Uint256
Expand All @@ -19,4 +19,60 @@ var _ = Describe("Storage value metadata getter", func() {
}
Expect(utils.GetStorageValueMetadata(metadataName, metadataKeys, metadataType)).To(Equal(expectedMetadata))
})

Describe("metadata for a packed storaged slot", func() {
It("returns metadata for multiple storage variables", func() {
metadataName := "fake_name"
metadataKeys := map[utils.Key]string{"key": "value"}
metadataType := utils.PackedSlot
metadataPackedNames := map[int]string{0: "name"}
metadataPackedTypes := map[int]utils.ValueType{0: utils.Uint48}

expectedMetadata := utils.StorageValueMetadata{
Name: metadataName,
Keys: metadataKeys,
Type: metadataType,
PackedTypes: metadataPackedTypes,
PackedNames: metadataPackedNames,
}
Expect(utils.GetStorageValueMetadataForPackedSlot(metadataName, metadataKeys, metadataType, metadataPackedNames, metadataPackedTypes)).To(Equal(expectedMetadata))
})

It("panics if PackedTypes are nil when the type is PackedSlot", func() {
metadataName := "fake_name"
metadataKeys := map[utils.Key]string{"key": "value"}
metadataType := utils.PackedSlot
metadataPackedNames := map[int]string{0: "name"}

getMetadata := func() {
utils.GetStorageValueMetadataForPackedSlot(metadataName, metadataKeys, metadataType, metadataPackedNames, nil)
}
Expect(getMetadata).To(Panic())
})

It("panics if PackedNames are nil when the type is PackedSlot", func() {
metadataName := "fake_name"
metadataKeys := map[utils.Key]string{"key": "value"}
metadataType := utils.PackedSlot
metadataPackedTypes := map[int]utils.ValueType{0: utils.Uint48}

getMetadata := func() {
utils.GetStorageValueMetadataForPackedSlot(metadataName, metadataKeys, metadataType, nil, metadataPackedTypes)
}
Expect(getMetadata).To(Panic())
})

It("panics if valueType is not PackedSlot if PackedNames is populated", func() {
metadataName := "fake_name"
metadataKeys := map[utils.Key]string{"key": "value"}
metadataType := utils.Uint48
metadataPackedNames := map[int]string{0: "name"}
metadataPackedTypes := map[int]utils.ValueType{0: utils.Uint48}

getMetadata := func() {
utils.GetStorageValueMetadataForPackedSlot(metadataName, metadataKeys, metadataType, metadataPackedNames, metadataPackedTypes)
}
Expect(getMetadata).To(Panic())
})
})
})
1 change: 0 additions & 1 deletion pkg/history/populate_headers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,3 @@ func RetrieveAndUpdateHeaders(blockChain core.BlockChain, headerRepository datas
}
return len(blockNumbers), nil
}