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.

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

db and vault should treat a key's version generically #684

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

adecaro
Copy link
Contributor

@adecaro adecaro commented Sep 23, 2024

This PR addresses #665


func (r NamespaceReads) Equals(o NamespaceReads) error {
return entriesEqual(r, o, func(v, v2 TxPosition) bool {
return v.Block == v2.Block && v.TxNum == v2.TxNum
return entriesEqual(r, o, func(v, v2 Version) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

We can also do

return entriesEqual(r, o, bytes.Equal)

@@ -26,11 +27,17 @@ type VersionedRead struct {
TxNum TxNum
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should change also VersionedRead and VersionedValue to contain Version instead of block and txnum?

Copy link
Contributor

Choose a reason for hiding this comment

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

We could use a type

type Version struct {
Block BlockNum
TxNum TxNum
}

func VersionFromBytes(RawVersion) Version

func (v *Version) Bytes() RawVersion

This way we still have access to the block and txnum

MKey = string
RawValue = []byte
Metadata = map[MKey][]byte
RawVersion = []byte
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be [8]byte

@@ -26,11 +27,17 @@ type VersionedRead struct {
TxNum TxNum
}

type VersionedMetadata struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could reuse the type in driver.go

TxNum driver.TxNum
}

func (v VersionedMetaData) Version() []byte {
Copy link
Contributor

Choose a reason for hiding this comment

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

We can return driver.RawVersion and re-use the code above.

return buf
}

type VersionedMetaData struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could rename this to VersionedMetadata (case)

"github.com/hyperledger-labs/fabric-smart-client/platform/common/driver"
)

var zeroVersion = []byte{0, 0, 0, 0, 0, 0, 0, 0}
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure we can assume that the length will be always 8 unless we change the type. I would change the check bytes.Equal(zeroVersion, b) to collections.ContainsOnly(b, 0)

return false
}

func ToBytes(Block driver.BlockNum, TxNum driver.TxNum) []byte {
Copy link
Contributor

Choose a reason for hiding this comment

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

The return type is RawVersion?

Copy link
Member

Choose a reason for hiding this comment

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

If RawVersion is the type, this method would be called toRawVersion, I guess

return false
}

func ToBytes(Block driver.BlockNum, TxNum driver.TxNum) []byte {
Copy link
Member

Choose a reason for hiding this comment

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

If RawVersion is the type, this method would be called toRawVersion, I guess


func ToBytes(Block driver.BlockNum, TxNum driver.TxNum) []byte {
buf := make([]byte, 8)
binary.BigEndian.PutUint32(buf[:4], uint32(Block))
Copy link
Member

Choose a reason for hiding this comment

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

Don't we run into issues here if fabric is used? Block height in Fabric is still uint64? I am wondering if we need to fix the version size at all?

@@ -90,7 +91,7 @@ func (p *populator) Populate(rws *vault.ReadWriteSet, rwsetBytes []byte, namespa
bn = read.Version.BlockNum
txn = read.Version.TxNum
}
rws.ReadSet.Add(ns, read.Key, bn, txn)
rws.ReadSet.Add(ns, read.Key, fver.ToBytes(bn, txn))
Copy link
Member

Choose a reason for hiding this comment

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

I think the Fabric Platform should provide this function not core/generic/vault as this is Fabric specific. Other platforms may implement different formats.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants