Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
jonastheis committed Nov 8, 2023
1 parent 68a36c7 commit d8aaf2f
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 53 deletions.
66 changes: 31 additions & 35 deletions pkg/model/commitment.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,45 +50,41 @@ func CommitmentFromCommitment(iotaCommitment *iotago.Commitment, api iotago.API,
return newCommitment(commitmentID, iotaCommitment, data, api)
}

func CommitmentFromBytesFactory(apiProvider iotago.APIProvider, opts ...serix.Option) func(bytes []byte) (*Commitment, int, error) {
func CommitmentFromBytes(apiProvider iotago.APIProvider) func([]byte) (*Commitment, int, error) {
return func(bytes []byte) (*Commitment, int, error) {
return CommitmentFromBytes(bytes, apiProvider, opts...)
totalBytesRead := 0

// We read the version byte here to determine the API to use, but then we decode the entire commitment again.
// Thus, we don't count the version byte as read bytes.
version, _, err := iotago.VersionFromBytes(bytes)
if err != nil {
return nil, 0, ierrors.Wrap(err, "failed to determine version")
}

apiForVersion, err := apiProvider.APIForVersion(version)
if err != nil {
return nil, 0, ierrors.Wrapf(err, "failed to get API for version %d", version)
}

iotaCommitment := new(iotago.Commitment)
if totalBytesRead, err = apiForVersion.Decode(bytes, iotaCommitment, serix.WithValidation()); err != nil {
return nil, 0, ierrors.Wrap(err, "failed to decode commitment")
}

commitmentID, err := iotaCommitment.ID()
if err != nil {
return nil, 0, ierrors.Wrap(err, "failed to determine commitment ID")
}

commitment, err := newCommitment(commitmentID, iotaCommitment, bytes, apiForVersion)
if err != nil {
return nil, 0, ierrors.Wrap(err, "failed to create commitment")
}

return commitment, totalBytesRead, nil
}
}

func CommitmentFromBytes(data []byte, apiProvider iotago.APIProvider, opts ...serix.Option) (*Commitment, int, error) {
totalBytesRead := 0

// We read the version byte here to determine the API to use, but then we decode the entire commitment again.
// Thus, we don't count the version byte as read bytes.
version, _, err := iotago.VersionFromBytes(data)
if err != nil {
return nil, 0, ierrors.Wrap(err, "failed to determine version")
}

apiForVersion, err := apiProvider.APIForVersion(version)
if err != nil {
return nil, 0, ierrors.Wrapf(err, "failed to get API for version %d", version)
}

iotaCommitment := new(iotago.Commitment)
if totalBytesRead, err = apiForVersion.Decode(data, iotaCommitment, opts...); err != nil {
return nil, 0, ierrors.Wrap(err, "failed to decode commitment")
}

commitmentID, err := iotaCommitment.ID()
if err != nil {
return nil, 0, ierrors.Wrap(err, "failed to determine commitment ID")
}

commitment, err := newCommitment(commitmentID, iotaCommitment, data, apiForVersion)
if err != nil {
return nil, 0, ierrors.Wrap(err, "failed to create commitment")
}

return commitment, totalBytesRead, nil
}

func (c *Commitment) ID() iotago.CommitmentID {
return c.commitmentID
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/model/validator_performance.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ type ValidatorPerformance struct {
// works if ValidatorBlocksPerSlot is less than 32 because we use it as bit vector
SlotActivityVector uint32
// can be uint8 because max count per slot is maximally ValidatorBlocksPerSlot + 1
BlockIssuedCount uint8
BlocksIssuedCount uint8
HighestSupportedVersionAndHash VersionAndHash
}

func NewValidatorPerformance() *ValidatorPerformance {
return &ValidatorPerformance{
SlotActivityVector: 0,
BlockIssuedCount: 0,
BlocksIssuedCount: 0,
HighestSupportedVersionAndHash: VersionAndHash{},
}
}
Expand All @@ -41,8 +41,8 @@ func ValidatorPerformanceFromReader(reader io.ReadSeeker) (*ValidatorPerformance
if v.SlotActivityVector, err = stream.Read[uint32](reader); err != nil {
return nil, ierrors.Wrap(err, "failed to read SlotActivityVector")
}
if v.BlockIssuedCount, err = stream.Read[uint8](reader); err != nil {
return nil, ierrors.Wrap(err, "failed to read BlockIssuedCount")
if v.BlocksIssuedCount, err = stream.Read[uint8](reader); err != nil {
return nil, ierrors.Wrap(err, "failed to read BlocksIssuedCount")
}
if v.HighestSupportedVersionAndHash, err = stream.ReadObject(reader, VersionAndHashSize, VersionAndHashFromBytes); err != nil {
return nil, ierrors.Wrap(err, "failed to read HighestSupportedVersionAndHash")
Expand All @@ -57,8 +57,8 @@ func (p *ValidatorPerformance) Bytes() ([]byte, error) {
if err := stream.Write(byteBuffer, p.SlotActivityVector); err != nil {
return nil, ierrors.Wrap(err, "failed to write SlotActivityVector")
}
if err := stream.Write(byteBuffer, p.BlockIssuedCount); err != nil {
return nil, ierrors.Wrap(err, "failed to write BlockIssuedCount")
if err := stream.Write(byteBuffer, p.BlocksIssuedCount); err != nil {
return nil, ierrors.Wrap(err, "failed to write BlocksIssuedCount")
}
if err := stream.WriteObject(byteBuffer, p.HighestSupportedVersionAndHash, VersionAndHash.Bytes); err != nil {
return nil, ierrors.Wrap(err, "failed to write HighestSupportedVersionAndHash")
Expand Down
5 changes: 2 additions & 3 deletions pkg/network/protocols/core/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"github.com/iotaledger/hive.go/runtime/syncutils"
"github.com/iotaledger/hive.go/runtime/workerpool"
"github.com/iotaledger/hive.go/serializer/v2"
"github.com/iotaledger/hive.go/serializer/v2/serix"
"github.com/iotaledger/hive.go/serializer/v2/stream"
"github.com/iotaledger/iota-core/pkg/model"
"github.com/iotaledger/iota-core/pkg/network"
Expand Down Expand Up @@ -178,7 +177,7 @@ func (p *Protocol) onBlockRequest(idBytes []byte, id peer.ID) {
}

func (p *Protocol) onSlotCommitment(commitmentBytes []byte, id peer.ID) {
receivedCommitment, _, err := model.CommitmentFromBytes(commitmentBytes, p.apiProvider, serix.WithValidation())
receivedCommitment, err := lo.DropCount(model.CommitmentFromBytes(p.apiProvider)(commitmentBytes))
if err != nil {
p.Events.Error.Trigger(ierrors.Wrap(err, "failed to deserialize slot commitment"), id)

Expand All @@ -199,7 +198,7 @@ func (p *Protocol) onSlotCommitmentRequest(idBytes []byte, id peer.ID) {
}

func (p *Protocol) onAttestations(commitmentBytes []byte, attestationsBytes []byte, merkleProof []byte, id peer.ID) {
cm, _, err := model.CommitmentFromBytes(commitmentBytes, p.apiProvider, serix.WithValidation())
cm, err := lo.DropCount(model.CommitmentFromBytes(p.apiProvider)(commitmentBytes))
if err != nil {
p.Events.Error.Trigger(ierrors.Wrap(err, "failed to deserialize commitment"), id)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ func (t *Tracker) aggregatePerformanceFactors(slotActivityVector []*model.Valida
// we reward not only total number of blocks issued, but also regularity based on block timestamp
slotPerformanceFactor := bits.OnesCount32(pf.SlotActivityVector)

if pf.BlockIssuedCount > protoParamsForEpoch.ValidationBlocksPerSlot() {
if pf.BlocksIssuedCount > protoParamsForEpoch.ValidationBlocksPerSlot() {
// we harshly punish validators that issue any blocks more than allowed

return 0
Expand Down Expand Up @@ -345,8 +345,8 @@ func (t *Tracker) trackCommitteeMemberPerformance(validationBlock *iotago.Valida

// we restrict the number up to ValidatorBlocksPerSlot + 1 to know later if the validator issued more blocks than allowed and be able to punish for it
// also it can fint into uint8
if validatorPerformance.BlockIssuedCount < apiForSlot.ProtocolParameters().ValidationBlocksPerSlot()+1 {
validatorPerformance.BlockIssuedCount++
if validatorPerformance.BlocksIssuedCount < apiForSlot.ProtocolParameters().ValidationBlocksPerSlot()+1 {
validatorPerformance.BlocksIssuedCount++
}

validatorPerformance.HighestSupportedVersionAndHash = model.VersionAndHash{
Expand Down
6 changes: 2 additions & 4 deletions pkg/storage/permanent/commitments.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func NewCommitments(store kvstore.KVStore, apiProvider iotago.APIProvider) *Comm
iotago.SlotIndex.Bytes,
iotago.SlotIndexFromBytes,
(*model.Commitment).Bytes,
model.CommitmentFromBytesFactory(apiProvider),
model.CommitmentFromBytes(apiProvider),
),
}
}
Expand Down Expand Up @@ -63,9 +63,7 @@ func (c *Commitments) Export(writer io.WriteSeeker, targetSlot iotago.SlotIndex)

func (c *Commitments) Import(reader io.ReadSeeker) (err error) {
if err := stream.ReadCollection(reader, serializer.SeriLengthPrefixTypeAsUint32, func(i int) error {
commitment, err := stream.ReadObjectWithSize[*model.Commitment](reader, serializer.SeriLengthPrefixTypeAsUint16, func(bytes []byte) (*model.Commitment, int, error) {
return model.CommitmentFromBytes(bytes, c.apiProvider)
})
commitment, err := stream.ReadObjectWithSize[*model.Commitment](reader, serializer.SeriLengthPrefixTypeAsUint16, model.CommitmentFromBytes(c.apiProvider))
if err != nil {
return ierrors.Wrapf(err, "failed to read commitment at index %d", i)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/storage/permanent/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func NewSettings(store kvstore.KVStore, opts ...options.Option[api.EpochBasedPro
store,
[]byte{latestCommitmentKey},
(*model.Commitment).Bytes,
model.CommitmentFromBytesFactory(apiProvider),
model.CommitmentFromBytes(apiProvider),
),
storeLatestFinalizedSlot: kvstore.NewTypedValue(
store,
Expand Down Expand Up @@ -542,7 +542,7 @@ func (s *Settings) Import(reader io.ReadSeeker) (err error) {
}

// Now that we parsed the protocol parameters, we can parse the commitment since there will be an API available
commitment, _, err := model.CommitmentFromBytes(commitmentBytes, s.apiProvider)
commitment, err := lo.DropCount(model.CommitmentFromBytes(s.apiProvider)(commitmentBytes))
if err != nil {
return ierrors.Wrap(err, "failed to parse commitment")
}
Expand Down

0 comments on commit d8aaf2f

Please sign in to comment.