From 07670726c824e8228503a2799efc7183db73dab5 Mon Sep 17 00:00:00 2001 From: hopeyen <60078528+hopeyen@users.noreply.github.com> Date: Tue, 17 Dec 2024 00:33:51 +0000 Subject: [PATCH 1/5] fix: properly set fields without onchain correspondance --- api/clients/v2/accountant.go | 95 ++++++++++++++++++-------------- core/data.go | 16 ++++++ core/meterer/offchain_store.go | 2 +- disperser/apiserver/server_v2.go | 10 ++-- 4 files changed, 76 insertions(+), 47 deletions(-) diff --git a/api/clients/v2/accountant.go b/api/clients/v2/accountant.go index 6b923d51b4..a9a3d7b11f 100644 --- a/api/clients/v2/accountant.go +++ b/api/clients/v2/accountant.go @@ -39,6 +39,13 @@ type BinRecord struct { Usage uint64 } +func DummyBinRecord() *BinRecord { + return &BinRecord{ + Index: 0, + Usage: 0, + } +} + func NewAccountant(accountID string, reservation *core.ReservedPayment, onDemand *core.OnDemandPayment, reservationWindow uint32, pricePerSymbol uint32, minNumSymbols uint32, numBins uint32) *Accountant { //TODO: client storage; currently every instance starts fresh but on-chain or a small store makes more sense // Also client is currently responsible for supplying network params, we need to add RPC in order to be automatic @@ -159,51 +166,55 @@ func (a *Accountant) SetPaymentState(paymentState *disperser_rpc.GetPaymentState return fmt.Errorf("payment state cannot be nil") } else if paymentState.GetPaymentGlobalParams() == nil { return fmt.Errorf("payment global params cannot be nil") - } else if paymentState.GetOnchainCumulativePayment() == nil { - return fmt.Errorf("onchain cumulative payment cannot be nil") - } else if paymentState.GetCumulativePayment() == nil { - return fmt.Errorf("cumulative payment cannot be nil") - } else if paymentState.GetReservation() == nil { - return fmt.Errorf("reservation cannot be nil") - } else if paymentState.GetReservation().GetQuorumNumbers() == nil { - return fmt.Errorf("reservation quorum numbers cannot be nil") - } else if paymentState.GetReservation().GetQuorumSplits() == nil { - return fmt.Errorf("reservation quorum split cannot be nil") - } else if paymentState.GetBinRecords() == nil { - return fmt.Errorf("bin records cannot be nil") - } - - a.minNumSymbols = uint32(paymentState.PaymentGlobalParams.MinNumSymbols) - a.onDemand.CumulativePayment = new(big.Int).SetBytes(paymentState.OnchainCumulativePayment) - a.cumulativePayment = new(big.Int).SetBytes(paymentState.CumulativePayment) - a.pricePerSymbol = uint32(paymentState.PaymentGlobalParams.PricePerSymbol) - - a.reservation.SymbolsPerSecond = uint64(paymentState.PaymentGlobalParams.GlobalSymbolsPerSecond) - a.reservation.StartTimestamp = uint64(paymentState.Reservation.StartTimestamp) - a.reservation.EndTimestamp = uint64(paymentState.Reservation.EndTimestamp) - a.reservationWindow = uint32(paymentState.PaymentGlobalParams.ReservationWindow) - - quorumNumbers := make([]uint8, len(paymentState.Reservation.QuorumNumbers)) - for i, quorum := range paymentState.Reservation.QuorumNumbers { - quorumNumbers[i] = uint8(quorum) - } - a.reservation.QuorumNumbers = quorumNumbers - - quorumSplits := make([]uint8, len(paymentState.Reservation.QuorumSplits)) - for i, quorum := range paymentState.Reservation.QuorumSplits { - quorumSplits[i] = uint8(quorum) - } - a.reservation.QuorumSplits = quorumSplits - - binRecords := make([]BinRecord, len(paymentState.BinRecords)) - for i, record := range paymentState.BinRecords { - binRecords[i] = BinRecord{ - Index: record.Index, - Usage: record.Usage, + } + + a.minNumSymbols = uint32(paymentState.GetPaymentGlobalParams().GetMinNumSymbols()) + a.pricePerSymbol = uint32(paymentState.GetPaymentGlobalParams().GetPricePerSymbol()) + a.reservationWindow = uint32(paymentState.GetPaymentGlobalParams().GetReservationWindow()) + + if paymentState.GetOnchainCumulativePayment() == nil { + a.onDemand.CumulativePayment = big.NewInt(0) + } else { + a.onDemand.CumulativePayment = new(big.Int).SetBytes(paymentState.GetOnchainCumulativePayment()) + } + + if paymentState.GetCumulativePayment() == nil { + a.cumulativePayment = big.NewInt(0) + } else { + a.cumulativePayment = new(big.Int).SetBytes(paymentState.GetCumulativePayment()) + } + + if paymentState.GetReservation() == nil { + a.reservation = core.DummyReservedPayment() + } else { + a.reservation.SymbolsPerSecond = uint64(paymentState.GetReservation().GetSymbolsPerSecond()) + a.reservation.StartTimestamp = uint64(paymentState.GetReservation().GetStartTimestamp()) + a.reservation.EndTimestamp = uint64(paymentState.GetReservation().GetEndTimestamp()) + quorumNumbers := make([]uint8, len(paymentState.GetReservation().GetQuorumNumbers())) + for i, quorum := range paymentState.GetReservation().GetQuorumNumbers() { + quorumNumbers[i] = uint8(quorum) + } + a.reservation.QuorumNumbers = quorumNumbers + + quorumSplits := make([]uint8, len(paymentState.GetReservation().GetQuorumSplits())) + for i, quorum := range paymentState.GetReservation().GetQuorumSplits() { + quorumSplits[i] = uint8(quorum) } + a.reservation.QuorumSplits = quorumSplits } - a.binRecords = binRecords + binRecords := make([]BinRecord, len(paymentState.GetBinRecords())) + for i, record := range paymentState.GetBinRecords() { + if record == nil { + binRecords[i] = *DummyBinRecord() + } else { + binRecords[i] = BinRecord{ + Index: record.Index, + Usage: record.Usage, + } + } + } + a.binRecords = binRecords return nil } diff --git a/core/data.go b/core/data.go index 367faad32d..0308230dd2 100644 --- a/core/data.go +++ b/core/data.go @@ -619,11 +619,27 @@ type ReservedPayment struct { QuorumSplits []byte } +func DummyReservedPayment() *ReservedPayment { + return &ReservedPayment{ + SymbolsPerSecond: 0, + StartTimestamp: 0, + EndTimestamp: 0, + QuorumNumbers: []uint8{}, + QuorumSplits: []byte{}, + } +} + type OnDemandPayment struct { // Total amount deposited by the user CumulativePayment *big.Int } +func DummyOnDemandPayment() *OnDemandPayment { + return &OnDemandPayment{ + CumulativePayment: big.NewInt(0), + } +} + type BlobVersionParameters struct { CodingRate uint32 MaxNumOperators uint32 diff --git a/core/meterer/offchain_store.go b/core/meterer/offchain_store.go index 6b213a495e..5899ca75e9 100644 --- a/core/meterer/offchain_store.go +++ b/core/meterer/offchain_store.go @@ -295,7 +295,7 @@ func (s *OffchainStore) GetLargestCumulativePayment(ctx context.Context, account } if len(payments) == 0 { - return nil, nil + return big.NewInt(0), nil } var payment *big.Int diff --git a/disperser/apiserver/server_v2.go b/disperser/apiserver/server_v2.go index bd45248f1d..6de37ec3be 100644 --- a/disperser/apiserver/server_v2.go +++ b/disperser/apiserver/server_v2.go @@ -254,20 +254,22 @@ func (s *DispersalServerV2) GetPaymentState(ctx context.Context, req *pb.GetPaym currentReservationPeriod := meterer.GetReservationPeriod(now, reservationWindow) binRecords, err := s.meterer.OffchainStore.GetBinRecords(ctx, req.AccountId, currentReservationPeriod) if err != nil { - return nil, api.NewErrorNotFound("failed to get active reservation") + s.logger.Debug("failed to get reservation records, use placeholders", err, accountID) } largestCumulativePayment, err := s.meterer.OffchainStore.GetLargestCumulativePayment(ctx, req.AccountId) if err != nil { - return nil, api.NewErrorNotFound("failed to get largest cumulative payment") + s.logger.Debug("failed to get largest cumulative payment, use zero value", err) } // on-Chain account state reservation, err := s.meterer.ChainPaymentState.GetReservedPaymentByAccount(ctx, accountID) if err != nil { - return nil, api.NewErrorNotFound("failed to get active reservation") + s.logger.Debug("failed to get onchain reservation, use zero values", err) + reservation = core.DummyReservedPayment() } onDemandPayment, err := s.meterer.ChainPaymentState.GetOnDemandPaymentByAccount(ctx, accountID) if err != nil { - return nil, api.NewErrorNotFound("failed to get on-demand payment") + s.logger.Debug("failed to get ondemand payment, use zero value", err) + onDemandPayment = core.DummyOnDemandPayment() } paymentGlobalParams := pb.PaymentGlobalParams{ From f9ed54e5884216089b2f086a8498cdc3f16167b9 Mon Sep 17 00:00:00 2001 From: hopeyen Date: Tue, 17 Dec 2024 11:48:14 -0800 Subject: [PATCH 2/5] refactor: comments and nit --- api/clients/v2/accountant.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/api/clients/v2/accountant.go b/api/clients/v2/accountant.go index a9a3d7b11f..0eb613b82b 100644 --- a/api/clients/v2/accountant.go +++ b/api/clients/v2/accountant.go @@ -39,13 +39,6 @@ type BinRecord struct { Usage uint64 } -func DummyBinRecord() *BinRecord { - return &BinRecord{ - Index: 0, - Usage: 0, - } -} - func NewAccountant(accountID string, reservation *core.ReservedPayment, onDemand *core.OnDemandPayment, reservationWindow uint32, pricePerSymbol uint32, minNumSymbols uint32, numBins uint32) *Accountant { //TODO: client storage; currently every instance starts fresh but on-chain or a small store makes more sense // Also client is currently responsible for supplying network params, we need to add RPC in order to be automatic @@ -161,6 +154,12 @@ func (a *Accountant) GetRelativeBinRecord(index uint32) *BinRecord { return &a.binRecords[relativeIndex] } +// SetPaymentState sets the accountant's state from the disperser's response +// We require disperser to return a valid set of global parameters, but optional +// account level on/off-chain state. If on-chain fields are not present, we use +// dummy values that disable accountant from using the corresponding payment method. +// If off-chain fields are not present, we assume the account has no payment history +// and set accoutant state to use initial values. func (a *Accountant) SetPaymentState(paymentState *disperser_rpc.GetPaymentStateReply) error { if paymentState == nil { return fmt.Errorf("payment state cannot be nil") @@ -206,7 +205,7 @@ func (a *Accountant) SetPaymentState(paymentState *disperser_rpc.GetPaymentState binRecords := make([]BinRecord, len(paymentState.GetBinRecords())) for i, record := range paymentState.GetBinRecords() { if record == nil { - binRecords[i] = *DummyBinRecord() + binRecords[i] = BinRecord{Index: 0, Usage: 0} } else { binRecords[i] = BinRecord{ Index: record.Index, From da716de798c20de501ae39d976f383f714d3c3a8 Mon Sep 17 00:00:00 2001 From: hopeyen Date: Tue, 17 Dec 2024 12:19:11 -0800 Subject: [PATCH 3/5] refactor: more explicite nil value handling --- disperser/apiserver/server_v2.go | 49 +++++++++++++++++++------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/disperser/apiserver/server_v2.go b/disperser/apiserver/server_v2.go index 6de37ec3be..88a226495a 100644 --- a/disperser/apiserver/server_v2.go +++ b/disperser/apiserver/server_v2.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "math/big" "net" "sync/atomic" "time" @@ -261,15 +262,37 @@ func (s *DispersalServerV2) GetPaymentState(ctx context.Context, req *pb.GetPaym s.logger.Debug("failed to get largest cumulative payment, use zero value", err) } // on-Chain account state + pbReservation := &pb.Reservation{} reservation, err := s.meterer.ChainPaymentState.GetReservedPaymentByAccount(ctx, accountID) if err != nil { s.logger.Debug("failed to get onchain reservation, use zero values", err) - reservation = core.DummyReservedPayment() + pbReservation = nil + } else { + quorumNumbers := make([]uint32, len(reservation.QuorumNumbers)) + for i, v := range reservation.QuorumNumbers { + quorumNumbers[i] = uint32(v) + } + quorumSplits := make([]uint32, len(reservation.QuorumSplits)) + for i, v := range reservation.QuorumSplits { + quorumSplits[i] = uint32(v) + } + + pbReservation = &pb.Reservation{ + SymbolsPerSecond: reservation.SymbolsPerSecond, + StartTimestamp: uint32(reservation.StartTimestamp), + EndTimestamp: uint32(reservation.EndTimestamp), + QuorumSplits: quorumSplits, + QuorumNumbers: quorumNumbers, + } } + + var onchainCumulativePayment *big.Int onDemandPayment, err := s.meterer.ChainPaymentState.GetOnDemandPaymentByAccount(ctx, accountID) if err != nil { s.logger.Debug("failed to get ondemand payment, use zero value", err) - onDemandPayment = core.DummyOnDemandPayment() + onchainCumulativePayment = nil + } else { + onchainCumulativePayment = onDemandPayment.CumulativePayment } paymentGlobalParams := pb.PaymentGlobalParams{ @@ -279,27 +302,13 @@ func (s *DispersalServerV2) GetPaymentState(ctx context.Context, req *pb.GetPaym ReservationWindow: reservationWindow, } - quorumNumbers := make([]uint32, len(reservation.QuorumNumbers)) - for i, v := range reservation.QuorumNumbers { - quorumNumbers[i] = uint32(v) - } - quorumSplits := make([]uint32, len(reservation.QuorumSplits)) - for i, v := range reservation.QuorumSplits { - quorumSplits[i] = uint32(v) - } // build reply reply := &pb.GetPaymentStateReply{ - PaymentGlobalParams: &paymentGlobalParams, - BinRecords: binRecords[:], - Reservation: &pb.Reservation{ - SymbolsPerSecond: reservation.SymbolsPerSecond, - StartTimestamp: uint32(reservation.StartTimestamp), - EndTimestamp: uint32(reservation.EndTimestamp), - QuorumSplits: quorumSplits, - QuorumNumbers: quorumNumbers, - }, + PaymentGlobalParams: &paymentGlobalParams, + BinRecords: binRecords[:], + Reservation: pbReservation, CumulativePayment: largestCumulativePayment.Bytes(), - OnchainCumulativePayment: onDemandPayment.CumulativePayment.Bytes(), + OnchainCumulativePayment: onchainCumulativePayment.Bytes(), } return reply, nil } From 6dfb53b76ef18b37738ef8e6295e5c7c009de830 Mon Sep 17 00:00:00 2001 From: hopeyen Date: Tue, 17 Dec 2024 12:25:54 -0800 Subject: [PATCH 4/5] refactor: logs and var declaration --- disperser/apiserver/server_v2.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/disperser/apiserver/server_v2.go b/disperser/apiserver/server_v2.go index 88a226495a..5259547167 100644 --- a/disperser/apiserver/server_v2.go +++ b/disperser/apiserver/server_v2.go @@ -255,18 +255,17 @@ func (s *DispersalServerV2) GetPaymentState(ctx context.Context, req *pb.GetPaym currentReservationPeriod := meterer.GetReservationPeriod(now, reservationWindow) binRecords, err := s.meterer.OffchainStore.GetBinRecords(ctx, req.AccountId, currentReservationPeriod) if err != nil { - s.logger.Debug("failed to get reservation records, use placeholders", err, accountID) + s.logger.Debug("failed to get reservation records, use placeholders", "err", err, "accountID", accountID) } largestCumulativePayment, err := s.meterer.OffchainStore.GetLargestCumulativePayment(ctx, req.AccountId) if err != nil { - s.logger.Debug("failed to get largest cumulative payment, use zero value", err) + s.logger.Debug("failed to get largest cumulative payment, use zero value", "err", err, "accountID", accountID) } // on-Chain account state - pbReservation := &pb.Reservation{} + var pbReservation *pb.Reservation reservation, err := s.meterer.ChainPaymentState.GetReservedPaymentByAccount(ctx, accountID) if err != nil { - s.logger.Debug("failed to get onchain reservation, use zero values", err) - pbReservation = nil + s.logger.Debug("failed to get onchain reservation, use zero values", "err", err, "accountID", accountID) } else { quorumNumbers := make([]uint32, len(reservation.QuorumNumbers)) for i, v := range reservation.QuorumNumbers { @@ -289,8 +288,7 @@ func (s *DispersalServerV2) GetPaymentState(ctx context.Context, req *pb.GetPaym var onchainCumulativePayment *big.Int onDemandPayment, err := s.meterer.ChainPaymentState.GetOnDemandPaymentByAccount(ctx, accountID) if err != nil { - s.logger.Debug("failed to get ondemand payment, use zero value", err) - onchainCumulativePayment = nil + s.logger.Debug("failed to get ondemand payment, use zero value", "err", err, "accountID", accountID) } else { onchainCumulativePayment = onDemandPayment.CumulativePayment } From b135abd6baf639a0638174687bc6db1ad73462b5 Mon Sep 17 00:00:00 2001 From: hopeyen Date: Tue, 17 Dec 2024 13:54:19 -0800 Subject: [PATCH 5/5] refactor: remove dummy structs --- api/clients/v2/accountant.go | 8 +++++++- core/data.go | 16 ---------------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/api/clients/v2/accountant.go b/api/clients/v2/accountant.go index 0eb613b82b..f5e1c920fa 100644 --- a/api/clients/v2/accountant.go +++ b/api/clients/v2/accountant.go @@ -184,7 +184,13 @@ func (a *Accountant) SetPaymentState(paymentState *disperser_rpc.GetPaymentState } if paymentState.GetReservation() == nil { - a.reservation = core.DummyReservedPayment() + a.reservation = &core.ReservedPayment{ + SymbolsPerSecond: 0, + StartTimestamp: 0, + EndTimestamp: 0, + QuorumNumbers: []uint8{}, + QuorumSplits: []byte{}, + } } else { a.reservation.SymbolsPerSecond = uint64(paymentState.GetReservation().GetSymbolsPerSecond()) a.reservation.StartTimestamp = uint64(paymentState.GetReservation().GetStartTimestamp()) diff --git a/core/data.go b/core/data.go index 0308230dd2..367faad32d 100644 --- a/core/data.go +++ b/core/data.go @@ -619,27 +619,11 @@ type ReservedPayment struct { QuorumSplits []byte } -func DummyReservedPayment() *ReservedPayment { - return &ReservedPayment{ - SymbolsPerSecond: 0, - StartTimestamp: 0, - EndTimestamp: 0, - QuorumNumbers: []uint8{}, - QuorumSplits: []byte{}, - } -} - type OnDemandPayment struct { // Total amount deposited by the user CumulativePayment *big.Int } -func DummyOnDemandPayment() *OnDemandPayment { - return &OnDemandPayment{ - CumulativePayment: big.NewInt(0), - } -} - type BlobVersionParameters struct { CodingRate uint32 MaxNumOperators uint32