From d8aaf2f7dedf5b246b40913afb8ef64b3c00209f Mon Sep 17 00:00:00 2001 From: jonastheis <4181434+jonastheis@users.noreply.github.com> Date: Wed, 8 Nov 2023 14:17:18 +0800 Subject: [PATCH] Address review comments --- pkg/model/commitment.go | 66 +++++++++---------- pkg/model/validator_performance.go | 12 ++-- pkg/network/protocols/core/protocol.go | 5 +- .../performance/performance.go | 6 +- pkg/storage/permanent/commitments.go | 6 +- pkg/storage/permanent/settings.go | 4 +- 6 files changed, 46 insertions(+), 53 deletions(-) diff --git a/pkg/model/commitment.go b/pkg/model/commitment.go index bade9cb63..d7cafb979 100644 --- a/pkg/model/commitment.go +++ b/pkg/model/commitment.go @@ -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 } diff --git a/pkg/model/validator_performance.go b/pkg/model/validator_performance.go index 4569f4f65..c121cfc23 100644 --- a/pkg/model/validator_performance.go +++ b/pkg/model/validator_performance.go @@ -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{}, } } @@ -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") @@ -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") diff --git a/pkg/network/protocols/core/protocol.go b/pkg/network/protocols/core/protocol.go index ac9426f05..cdf696117 100644 --- a/pkg/network/protocols/core/protocol.go +++ b/pkg/network/protocols/core/protocol.go @@ -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" @@ -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) @@ -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) diff --git a/pkg/protocol/sybilprotection/sybilprotectionv1/performance/performance.go b/pkg/protocol/sybilprotection/sybilprotectionv1/performance/performance.go index f5f024a56..ceac2fdf5 100644 --- a/pkg/protocol/sybilprotection/sybilprotectionv1/performance/performance.go +++ b/pkg/protocol/sybilprotection/sybilprotectionv1/performance/performance.go @@ -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 @@ -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{ diff --git a/pkg/storage/permanent/commitments.go b/pkg/storage/permanent/commitments.go index 430f13288..4f832ee22 100644 --- a/pkg/storage/permanent/commitments.go +++ b/pkg/storage/permanent/commitments.go @@ -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), ), } } @@ -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) } diff --git a/pkg/storage/permanent/settings.go b/pkg/storage/permanent/settings.go index cca9807cd..be4459ed1 100644 --- a/pkg/storage/permanent/settings.go +++ b/pkg/storage/permanent/settings.go @@ -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, @@ -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") }