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

fix: properly set fields without onchain correspondance #1014

Merged
merged 5 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
94 changes: 52 additions & 42 deletions api/clients/v2/accountant.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,56 +154,66 @@ 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")
} 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,
}

Copy link
Contributor

Choose a reason for hiding this comment

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

can we document this behavior (which fields are optional and what the consequences are)?

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()
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: could we explicitly create empty object

a.reservation = &ReservedPayment{
	SymbolsPerSecond: 0,
	StartTimestamp:   0,
	EndTimestamp:     0,
	QuorumNumbers:    []uint8{},
	QuorumSplits:     []byte{},
}

and remove DummyReservedPayment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, updated

} 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] = BinRecord{Index: 0, Usage: 0}
} else {
binRecords[i] = BinRecord{
Index: record.Index,
Usage: record.Usage,
}
}
}
a.binRecords = binRecords
return nil
}

Expand Down
16 changes: 16 additions & 0 deletions core/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

return &OnDemandPayment{
CumulativePayment: big.NewInt(0),
}
}

type BlobVersionParameters struct {
CodingRate uint32
MaxNumOperators uint32
Expand Down
2 changes: 1 addition & 1 deletion core/meterer/offchain_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
55 changes: 33 additions & 22 deletions disperser/apiserver/server_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"context"
"errors"
"fmt"
"math/big"
"net"
"sync/atomic"
"time"
Expand Down Expand Up @@ -254,20 +255,44 @@
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)
Copy link
Contributor

Choose a reason for hiding this comment

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

use labels:

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 {
return nil, api.NewErrorNotFound("failed to get largest cumulative payment")
s.logger.Debug("failed to get largest cumulative payment, use zero value", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think it would be helpful to log accountIDs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated to include accountIDs and use labels

}
// on-Chain account state
pbReservation := &pb.Reservation{}

Check failure on line 265 in disperser/apiserver/server_v2.go

View workflow job for this annotation

GitHub Actions / Linter

ineffectual assignment to pbReservation (ineffassign)

Check failure on line 265 in disperser/apiserver/server_v2.go

View workflow job for this annotation

GitHub Actions / Linter

ineffectual assignment to pbReservation (ineffassign)
Copy link
Contributor

Choose a reason for hiding this comment

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

if we declare it like var pbReservation *pb.Reservation, we don't have to set pbReservation = nil in L269

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)
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 {
return nil, api.NewErrorNotFound("failed to get on-demand payment")
s.logger.Debug("failed to get ondemand payment, use zero value", err)
onchainCumulativePayment = nil
} else {
onchainCumulativePayment = onDemandPayment.CumulativePayment
}

paymentGlobalParams := pb.PaymentGlobalParams{
Expand All @@ -277,27 +302,13 @@
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
}
Loading