Skip to content
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.

Sign up for GitHub

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

Jump to bottom

UUID partition change #4914

Merged
merged 13 commits into from
Jan 24, 2024
Merged

UUID partition change #4914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c28cdda
change partition byte for uuid to 3rd byte
bluesign Oct 31, 2023
462b9cc
udid partition tests
bluesign Oct 31, 2023
20f0dc5
Update uuids.go
bluesign Nov 1, 2023
0d02488
Update fvm/environment/uuids.go
bluesign Nov 1, 2023
48520a1
continuation test
bluesign Nov 3, 2023
000ea72
test
bluesign Nov 3, 2023
2a27781
Update fvm/environment/uuids.go
bluesign Nov 3, 2023
efe1c6f
Update fvm/environment/uuids.go
bluesign Nov 3, 2023
8eb7df2
rename
bluesign Nov 3, 2023
2c71696
Merge branch 'uuid_partition_change' of https://github.com/bluesign/f…
bluesign Nov 3, 2023
4f4c564
rename methods to getCounter/setCounter
bluesign Nov 3, 2023
0fd79a8
Merge branch 'master' into uuid_partition_change
janezpodhostnik Jan 24, 2024
a3b336c
change expected state comitments
janezpodhostnik Jan 24, 2024
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
2 changes: 1 addition & 1 deletion engine/execution/state/bootstrap/bootstrap_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestBootstrapLedger(t *testing.T) {
}

func TestBootstrapLedger_ZeroTokenSupply(t *testing.T) {
expectedStateCommitmentBytes, _ := hex.DecodeString("4c5b099dae68a858dd8da0944e6fad6f6d1b943b83c5acb39aeee659e165adb5")
expectedStateCommitmentBytes, _ := hex.DecodeString("8d9d52a66a832898f6f2416b703759b7ecd1eb390db6d5e727c2daeec001ffc6")
expectedStateCommitment, err := flow.ToStateCommitment(expectedStateCommitmentBytes)
require.NoError(t, err)

Expand Down
34 changes: 24 additions & 10 deletions fvm/environment/uuids.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,20 @@ import (
"github.com/onflow/flow-go/utils/slices"
)

// uuid is partitioned with 3rd byte for compatibility reasons.
// (database types and Javascript safe integer limits)
//
// counter(C) is 7 bytes, paritition(P) is 1 byte
// uuid is assembled by first reading the counter from the register value of the partitioned register,
// and then left shifting the 6th and 7th byte, and placing the partition byte at 6th byte:
// C7 C6 P C5 C4 C3 C2 C1
bluesign marked this conversation as resolved.
Show resolved Hide resolved
//
// Until resource ids start filling the bits above the 48th one, dapps will have enough time
// to switch to a larger data type.

const (
// The max value for any is uuid partition is MaxUint56, since the top
// 8 bits in the uuid are used for partitioning.
// The max value for any is uuid partition is MaxUint56, since one byte
// in the uuid is used for partitioning.
MaxUint56 = (uint64(1) << 56) - 1

// Start warning when there's only a single high bit left. This should give
Expand Down Expand Up @@ -108,8 +119,8 @@ func NewUUIDGenerator(
}
}

// getUint64 reads the uint64 value from the partitioned uuid register.
func (generator *uUIDGenerator) getUint64() (uint64, error) {
// getCounter reads the uint64 value from the partitioned uuid register.
func (generator *uUIDGenerator) getCounter() (uint64, error) {
stateBytes, err := generator.txnState.Get(generator.registerId)
if err != nil {
return 0, fmt.Errorf(
Expand All @@ -122,8 +133,8 @@ func (generator *uUIDGenerator) getUint64() (uint64, error) {
return binary.BigEndian.Uint64(bytes), nil
}

// setUint56 sets a new uint56 value into the partitioned uuid register.
func (generator *uUIDGenerator) setUint56(
// setCounter sets a new uint56 value into the partitioned uuid register.
func (generator *uUIDGenerator) setCounter(
value uint64,
) error {
if value > Uint56OverflowWarningThreshold {
Expand Down Expand Up @@ -184,17 +195,20 @@ func (generator *uUIDGenerator) GenerateUUID() (uint64, error) {

generator.maybeInitializePartition()

value, err := generator.getUint64()
counter, err := generator.getCounter()
if err != nil {
return 0, fmt.Errorf("cannot generate UUID: %w", err)
}

err = generator.setUint56(value + 1)
err = generator.setCounter(counter + 1)
if err != nil {
return 0, fmt.Errorf("cannot generate UUID: %w", err)
}

// Since the partition counter only goes up to MaxUint56, we can use the
// upper 8 bits to represent which partition was used.
return (uint64(generator.partition) << 56) | value, nil
// assemble a UUID value with the partition (P) and the counter (C).
// Note: partition (P) is represented by the 6th byte
// (C7 C6) | P | (C5 C4 C3 C2 C1)
return ((counter & 0xFF_FF00_0000_0000) << 8) | (uint64(generator.partition) << 40) | (counter & 0xFF_FFFF_FFFF), nil

}
97 changes: 67 additions & 30 deletions fvm/environment/uuids_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,9 @@ func testUUIDGenerator(t *testing.T, blockHeader *flow.Header, txnIndex uint32)
generator.maybeInitializePartition()

partition := generator.partition
partitionMinValue := uint64(partition) << 56
maxUint56 := uint64(72057594037927935) // (1 << 56) - 1
partitionMinValue := uint64(partition) << 40
maxUint56 := uint64(0xFFFFFFFFFFFFFF)
maxUint56Split := uint64(0xFFFF00FFFFFFFFFF)

t.Run(
fmt.Sprintf("basic get and set uint (partition: %d)", partition),
Expand All @@ -131,11 +132,11 @@ func testUUIDGenerator(t *testing.T, blockHeader *flow.Header, txnIndex uint32)
txnIndex)
uuidsA.maybeInitializePartition()

uuid, err := uuidsA.getUint64() // start from zero
uuid, err := uuidsA.getCounter() // start from zero
require.NoError(t, err)
require.Equal(t, uint64(0), uuid)

err = uuidsA.setUint56(5)
err = uuidsA.setCounter(5)
require.NoError(t, err)

// create new UUIDs instance
Expand All @@ -148,7 +149,7 @@ func testUUIDGenerator(t *testing.T, blockHeader *flow.Header, txnIndex uint32)
txnIndex)
uuidsB.maybeInitializePartition()

uuid, err = uuidsB.getUint64() // should read saved value
uuid, err = uuidsB.getCounter() // should read saved value
require.NoError(t, err)

require.Equal(t, uint64(5), uuid)
Expand Down Expand Up @@ -204,7 +205,7 @@ func testUUIDGenerator(t *testing.T, blockHeader *flow.Header, txnIndex uint32)
})

t.Run(
fmt.Sprintf("setUint56 overflows (partition: %d)", partition),
fmt.Sprintf("setCounter overflows (partition: %d)", partition),
func(t *testing.T) {
txnState := state.NewTransactionState(nil, state.DefaultParameters())
uuids := NewUUIDGenerator(
Expand All @@ -216,17 +217,17 @@ func testUUIDGenerator(t *testing.T, blockHeader *flow.Header, txnIndex uint32)
txnIndex)
uuids.maybeInitializePartition()

err := uuids.setUint56(maxUint56)
err := uuids.setCounter(maxUint56)
require.NoError(t, err)

value, err := uuids.getUint64()
value, err := uuids.getCounter()
require.NoError(t, err)
require.Equal(t, value, maxUint56)

err = uuids.setUint56(maxUint56 + 1)
err = uuids.setCounter(maxUint56 + 1)
require.ErrorContains(t, err, "overflowed")

value, err = uuids.getUint64()
value, err = uuids.getCounter()
require.NoError(t, err)
require.Equal(t, value, maxUint56)
})
Expand All @@ -244,22 +245,22 @@ func testUUIDGenerator(t *testing.T, blockHeader *flow.Header, txnIndex uint32)
txnIndex)
uuids.maybeInitializePartition()

err := uuids.setUint56(maxUint56 - 1)
err := uuids.setCounter(maxUint56 - 1)
require.NoError(t, err)

value, err := uuids.GenerateUUID()
require.NoError(t, err)
require.Equal(t, value, partitionMinValue+maxUint56-1)
require.Equal(t, value, partitionMinValue|(maxUint56-1))
require.Equal(t, value, partitionMinValue+maxUint56Split-1)
require.Equal(t, value, partitionMinValue|(maxUint56Split-1))

value, err = uuids.getUint64()
value, err = uuids.getCounter()
require.NoError(t, err)
require.Equal(t, value, maxUint56)

_, err = uuids.GenerateUUID()
require.ErrorContains(t, err, "overflowed")

value, err = uuids.getUint64()
value, err = uuids.getCounter()
require.NoError(t, err)
require.Equal(t, value, maxUint56)
})
Expand All @@ -282,33 +283,33 @@ func TestUUIDGeneratorHardcodedPartitionIdGeneration(t *testing.T) {

value, err := uuids.GenerateUUID()
require.NoError(t, err)
require.Equal(t, value, uint64(0xde00000000000000))
require.Equal(t, value, uint64(0x0000de0000000000))

value, err = uuids.getUint64()
value, err = uuids.getCounter()
require.NoError(t, err)
require.Equal(t, value, uint64(1))

value, err = uuids.GenerateUUID()
require.NoError(t, err)
require.Equal(t, value, uint64(0xde00000000000001))
require.Equal(t, value, uint64(0x0000de0000000001))

value, err = uuids.getUint64()
value, err = uuids.getCounter()
require.NoError(t, err)
require.Equal(t, value, uint64(2))

value, err = uuids.GenerateUUID()
require.NoError(t, err)
require.Equal(t, value, uint64(0xde00000000000002))
require.Equal(t, value, uint64(0x0000de0000000002))

value, err = uuids.getUint64()
value, err = uuids.getCounter()
require.NoError(t, err)
require.Equal(t, value, uint64(3))

// pretend we increamented the counter up to cafBad
cafBad := uint64(0x1c2a3f4b5a6d70)
decafBad := uint64(0xde1c2a3f4b5a6d70)
decafBad := uint64(0x1c2ade3f4b5a6d70)

err = uuids.setUint56(cafBad)
err = uuids.setCounter(cafBad)
require.NoError(t, err)

for i := 0; i < 5; i++ {
Expand All @@ -317,35 +318,71 @@ func TestUUIDGeneratorHardcodedPartitionIdGeneration(t *testing.T) {
require.Equal(t, value, decafBad+uint64(i))
}

value, err = uuids.getUint64()
value, err = uuids.getCounter()
require.NoError(t, err)
require.Equal(t, value, cafBad+uint64(5))

// pretend we increamented the counter up to overflow - 2
maxUint56Minus2 := uint64(0xfffffffffffffd)
err = uuids.setUint56(maxUint56Minus2)
err = uuids.setCounter(maxUint56Minus2)
require.NoError(t, err)

value, err = uuids.GenerateUUID()
require.NoError(t, err)
require.Equal(t, value, uint64(0xdefffffffffffffd))
require.Equal(t, value, uint64(0xffffdefffffffffd))

value, err = uuids.getUint64()
value, err = uuids.getCounter()
require.NoError(t, err)
require.Equal(t, value, maxUint56Minus2+1)

value, err = uuids.GenerateUUID()
require.NoError(t, err)
require.Equal(t, value, uint64(0xdefffffffffffffe))
require.Equal(t, value, uint64(0xffffdefffffffffe))

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about the implementation.

We know that the UUID generator generates UUID for the same partition, and the partition is determined by the block ID and transaction index.

In other words, given a UUID Generator, if we keep calling GenerateUUID, the returned value always the same byte at the partition byte position, which is the 3rd byte.

However, L201 takes the stored uuid value and add 1 to it:

	err = generator.setUint56(value + 1)

This works if partition is the 1st byte, but won't work if partition is 3rd byte.
See this case: if now the UUID for partition 2 is: 3298534883327 (hex: 00 00 02 FF FF FF FF FF)

C7 C6  P C5 C4 C3 C2 C1
00 00 02 FF FF FF FF FF

Then calling GenerateUUID() again, will call setUint56(value + 1), which becomes 00 00 03 00 00 00 00 00, which would not increment C6, and causing the counter to go back to 0 again.

We need to have a test case, for a UUID generator for partition 2 and it's value is 00 00 02 FF FF FF FF FF, the next two UUIDs should be 00 01 02 00 00 00 00 00, and 00 01 02 00 00 00 00 01.

Copy link
Contributor

Choose a reason for hiding this comment

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

What happens is that we first call generator.getUint64() which gets the stored counter for that partition (00 00 00 FF FF FF FF FF). then stores it back as incremented by 1 generator.setUint56(value + 1) ( 00 00 01 00 00 00 00 00). Only after this is the value cut up and the partition bytes inserted.

value: 00 00 00 FF FF FF FF FF
partition: 02

(value & 0xFF_FF00_0000_0000) << 8) -> 00 00 00 00 00 00 00 00
(partition << 40)                   -> 00 00 02 00 00 00 00 00
(value & 0xFF_FFFF_FFFF)            -> 00 00 00 FF FF FF FF FF
                                       00 00 02 FF FF FF FF FF

and the next one:
value: 00 00 01 00 00 00 00 00
partition: 02

(value & 0xFF_FF00_0000_0000) << 8) -> 00 01 00 00 00 00 00 00
(partition << 40)                   -> 00 00 02 00 00 00 00 00
(value & 0xFF_FFFF_FFFF)            -> 00 00 00 00 00 00 00 00
                                       00 01 02 00 00 00 00 00

I do agree that a test for this continuation would be great!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zhangchiqing
actually problem is naming of the function, I didn't want to touch that. Actually getuint64 is just returning the counter ( without partition byte )

Copy link
Member

Choose a reason for hiding this comment

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

We need to have a test case, for a UUID generator for partition 2 and it's value is 00 00 02 FF FF FF FF FF, the next two UUIDs should be 00 01 02 00 00 00 00 00, and 00 01 02 00 00 00 00 01.

@bluesign could you at least add this test case? Just want to make sure we have test case covering it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, of course, I missed that part of the comment

Copy link
Member

@zhangchiqing zhangchiqing Nov 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, I know where I got confused. I thought the stored register value contains the partition and the counter, but actually it is just the counter. The register ID contains the partition already.

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 naming of the functions are confusing, setUint56 and getUint64 makes you feel like you are setting 56 bits of 64 bit int, and then getting it as 64 bit with partition.

Copy link
Member

Choose a reason for hiding this comment

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

exactly

value, err = uuids.getUint64()
value, err = uuids.getCounter()
require.NoError(t, err)
require.Equal(t, value, maxUint56Minus2+2)

_, err = uuids.GenerateUUID()
require.ErrorContains(t, err, "overflowed")

value, err = uuids.getUint64()
value, err = uuids.getCounter()
require.NoError(t, err)
require.Equal(t, value, maxUint56Minus2+2)
}

func TestContinuati(t *testing.T) {
txnState := state.NewTransactionState(nil, state.DefaultParameters())
uuids := NewUUIDGenerator(
tracing.NewTracerSpan(),
zerolog.Nop(),
NewMeter(txnState),
txnState,
nil,
0)

// Hardcoded the partition to check for exact bytes
uuids.initialized = true
uuids.partition = 0x01
uuids.registerId = flow.UUIDRegisterID(0x01)

value, err := uuids.GenerateUUID()
require.NoError(t, err)
require.Equal(t, value, uint64(0x0000010000000000))

err = uuids.setCounter(0xFFFFFFFFFF)
require.NoError(t, err)

value, err = uuids.GenerateUUID()
require.NoError(t, err)
require.Equal(t, value, uint64(0x000001FFFFFFFFFF))

value, err = uuids.GenerateUUID()
require.NoError(t, err)
require.Equal(t, value, uint64(0x0001010000000000))

value, err = uuids.GenerateUUID()
require.NoError(t, err)
require.Equal(t, value, uint64(0x0001010000000001))

}
6 changes: 3 additions & 3 deletions utils/unittest/execution_state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const ServiceAccountPrivateKeySignAlgo = crypto.ECDSAP256
const ServiceAccountPrivateKeyHashAlgo = hash.SHA2_256

// Pre-calculated state commitment with root account with the above private key
const GenesisStateCommitmentHex = "6c394b798bcabfdbdcfddb98f33a818de81efc160d99a4697db57b7b099d1ab1"
const GenesisStateCommitmentHex = "e4674bba14f59af783bbf70b2a43c1696a7d9888eeaca86cf74b033580fe1c23"

var GenesisStateCommitment flow.StateCommitment

Expand Down Expand Up @@ -87,10 +87,10 @@ func genesisCommitHexByChainID(chainID flow.ChainID) string {
return GenesisStateCommitmentHex
}
if chainID == flow.Testnet {
return "5dc11f195653540c1cc3c2fd42ac9d9dca415be6080276eebd1e2fa5dba07a1c"
return "bfe964655cf13711b93dbaf156aaebbc24a607beed69dd36d71b593832b5129c"
}
if chainID == flow.Sandboxnet {
return "e1c08b17f9e5896f03fe28dd37ca396c19b26628161506924fbf785834646ea1"
}
return "0d5dcd6cd42cbc41c2aae1a4a6ee950758cc2f75f21ad0ccf84b9e9fa35305ff"
return "a56a2750708bc981eb949a3b02a41061dc6b7e6bfa9f31a19a48f560f616bed3"
}
Loading