Skip to content

avoid unnecessary casting #1327

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Feb 28, 2025
Merged

avoid unnecessary casting #1327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
94b1722
refactor: payment params casting update
hopeyen Feb 25, 2025
ed06e20
refactor: reader casting
hopeyen Feb 25, 2025
db8dd25
Merge branch 'master' into hope/misc-casting
hopeyen Feb 27, 2025
39c3c74
chore: merge master resolve conflict
hopeyen Feb 27, 2025
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
32 changes: 16 additions & 16 deletions api/clients/v2/accountant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ type Accountant struct {
accountID string
reservation *core.ReservedPayment
onDemand *core.OnDemandPayment
reservationWindow uint32
pricePerSymbol uint32
minNumSymbols uint32
reservationWindow uint64
pricePerSymbol uint64
minNumSymbols uint64

// local accounting
// contains 3 bins; circular wrapping of indices
Expand All @@ -38,7 +38,7 @@ type PeriodRecord struct {
Usage uint64
}

func NewAccountant(accountID string, reservation *core.ReservedPayment, onDemand *core.OnDemandPayment, reservationWindow uint32, pricePerSymbol uint32, minNumSymbols uint32, numBins uint32) *Accountant {
func NewAccountant(accountID string, reservation *core.ReservedPayment, onDemand *core.OnDemandPayment, reservationWindow uint64, pricePerSymbol uint64, minNumSymbols uint64, numBins uint32) *Accountant {
periodRecords := make([]PeriodRecord, numBins)
for i := range periodRecords {
periodRecords[i] = PeriodRecord{Index: uint32(i), Usage: 0}
Expand Down Expand Up @@ -67,9 +67,9 @@ func NewAccountant(accountID string, reservation *core.ReservedPayment, onDemand
// indicating on-demand payment.
// These generated values are used to create the payment header and signature, as specified in
// api/proto/common/v2/common_v2.proto
func (a *Accountant) BlobPaymentInfo(ctx context.Context, numSymbols uint32, quorumNumbers []uint8, timestamp int64) (*big.Int, error) {
func (a *Accountant) BlobPaymentInfo(ctx context.Context, numSymbols uint64, quorumNumbers []uint8, timestamp int64) (*big.Int, error) {
currentReservationPeriod := meterer.GetReservationPeriodByNanosecond(timestamp, a.reservationWindow)
symbolUsage := uint64(a.SymbolsCharged(numSymbols))
symbolUsage := a.SymbolsCharged(numSymbols)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the a max value for numSymbols that we should be checking ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not enforced by payments, I think it will be checked in validation before it gets to meterer. If you think it is within the meterer scope and should double check, I can add config to the meterer for a maximum number of blobs and add a check

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's enforced somewhere else and it's the only call path then no need.


a.usageLock.Lock()
defer a.usageLock.Unlock()
Expand Down Expand Up @@ -111,7 +111,7 @@ func (a *Accountant) BlobPaymentInfo(ctx context.Context, numSymbols uint32, quo
}

// AccountBlob accountant provides and records payment information
func (a *Accountant) AccountBlob(ctx context.Context, timestamp int64, numSymbols uint32, quorums []uint8) (*core.PaymentMetadata, error) {
func (a *Accountant) AccountBlob(ctx context.Context, timestamp int64, numSymbols uint64, quorums []uint8) (*core.PaymentMetadata, error) {
cumulativePayment, err := a.BlobPaymentInfo(ctx, numSymbols, quorums, timestamp)
if err != nil {
return nil, err
Expand All @@ -128,22 +128,22 @@ func (a *Accountant) AccountBlob(ctx context.Context, timestamp int64, numSymbol

// TODO: PaymentCharged and SymbolsCharged copied from meterer, should be refactored
// PaymentCharged returns the chargeable price for a given data length
func (a *Accountant) PaymentCharged(numSymbols uint32) uint64 {
return uint64(a.SymbolsCharged(numSymbols)) * uint64(a.pricePerSymbol)
func (a *Accountant) PaymentCharged(numSymbols uint64) uint64 {
return a.SymbolsCharged(numSymbols) * a.pricePerSymbol
}

// SymbolsCharged returns the number of symbols charged for a given data length
// being at least MinNumSymbols or the nearest rounded-up multiple of MinNumSymbols.
func (a *Accountant) SymbolsCharged(numSymbols uint32) uint32 {
func (a *Accountant) SymbolsCharged(numSymbols uint64) uint64 {
if numSymbols <= a.minNumSymbols {
return a.minNumSymbols
}
// Round up to the nearest multiple of MinNumSymbols
return uint32(core.RoundUpDivide(uint(numSymbols), uint(a.minNumSymbols))) * a.minNumSymbols
return core.RoundUpDivide(numSymbols, a.minNumSymbols) * a.minNumSymbols
}

func (a *Accountant) GetRelativePeriodRecord(index uint32) *PeriodRecord {
relativeIndex := index % a.numBins
func (a *Accountant) GetRelativePeriodRecord(index uint64) *PeriodRecord {
relativeIndex := uint32(index % uint64(a.numBins))
if a.periodRecords[relativeIndex].Index != uint32(index) {
a.periodRecords[relativeIndex] = PeriodRecord{
Index: uint32(index),
Expand All @@ -167,9 +167,9 @@ func (a *Accountant) SetPaymentState(paymentState *disperser_rpc.GetPaymentState
return fmt.Errorf("payment global params cannot be nil")
}

a.minNumSymbols = uint32(paymentState.GetPaymentGlobalParams().GetMinNumSymbols())
a.pricePerSymbol = uint32(paymentState.GetPaymentGlobalParams().GetPricePerSymbol())
a.reservationWindow = uint32(paymentState.GetPaymentGlobalParams().GetReservationWindow())
a.minNumSymbols = paymentState.GetPaymentGlobalParams().GetMinNumSymbols()
a.pricePerSymbol = paymentState.GetPaymentGlobalParams().GetPricePerSymbol()
a.reservationWindow = paymentState.GetPaymentGlobalParams().GetReservationWindow()

if paymentState.GetOnchainCumulativePayment() == nil {
a.onDemand = &core.OnDemandPayment{
Expand Down
62 changes: 31 additions & 31 deletions api/clients/v2/accountant_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ func TestNewAccountant(t *testing.T) {
onDemand := &core.OnDemandPayment{
CumulativePayment: big.NewInt(500),
}
reservationWindow := uint32(6)
pricePerSymbol := uint32(1)
minNumSymbols := uint32(100)
reservationWindow := uint64(6)
pricePerSymbol := uint64(1)
minNumSymbols := uint64(100)

privateKey1, err := crypto.GenerateKey()
assert.NoError(t, err)
Expand Down Expand Up @@ -57,17 +57,17 @@ func TestAccountBlob_Reservation(t *testing.T) {
onDemand := &core.OnDemandPayment{
CumulativePayment: big.NewInt(500),
}
reservationWindow := uint32(5)
pricePerSymbol := uint32(1)
minNumSymbols := uint32(100)
reservationWindow := uint64(5)
pricePerSymbol := uint64(1)
minNumSymbols := uint64(100)

privateKey1, err := crypto.GenerateKey()
assert.NoError(t, err)
accountId := hex.EncodeToString(privateKey1.D.Bytes())
accountant := NewAccountant(accountId, reservation, onDemand, reservationWindow, pricePerSymbol, minNumSymbols, numBins)

ctx := context.Background()
symbolLength := uint32(500)
symbolLength := uint64(500)
quorums := []uint8{0, 1}
now := time.Now().UnixNano()

Expand All @@ -78,7 +78,7 @@ func TestAccountBlob_Reservation(t *testing.T) {
assert.Equal(t, big.NewInt(0), header.CumulativePayment)
assert.Equal(t, isRotation([]uint64{500, 0, 0}, mapRecordUsage(accountant.periodRecords)), true)

symbolLength = uint32(700)
symbolLength = uint64(700)

now = time.Now().UnixNano()
header, err = accountant.AccountBlob(ctx, now, symbolLength, quorums)
Expand Down Expand Up @@ -108,17 +108,17 @@ func TestAccountBlob_OnDemand(t *testing.T) {
onDemand := &core.OnDemandPayment{
CumulativePayment: big.NewInt(1500),
}
reservationWindow := uint32(5)
pricePerSymbol := uint32(1)
minNumSymbols := uint32(100)
reservationWindow := uint64(5)
pricePerSymbol := uint64(1)
minNumSymbols := uint64(100)

privateKey1, err := crypto.GenerateKey()
assert.NoError(t, err)
accountId := hex.EncodeToString(privateKey1.D.Bytes())
accountant := NewAccountant(accountId, reservation, onDemand, reservationWindow, pricePerSymbol, minNumSymbols, numBins)

ctx := context.Background()
numSymbols := uint32(1500)
numSymbols := uint64(1500)
quorums := []uint8{0, 1}
now := time.Now().UnixNano()
header, err := accountant.AccountBlob(ctx, now, numSymbols, quorums)
Expand All @@ -136,17 +136,17 @@ func TestAccountBlob_InsufficientOnDemand(t *testing.T) {
onDemand := &core.OnDemandPayment{
CumulativePayment: big.NewInt(500),
}
reservationWindow := uint32(60)
pricePerSymbol := uint32(100)
minNumSymbols := uint32(100)
reservationWindow := uint64(60)
pricePerSymbol := uint64(100)
minNumSymbols := uint64(100)

privateKey1, err := crypto.GenerateKey()
assert.NoError(t, err)
accountId := hex.EncodeToString(privateKey1.D.Bytes())
accountant := NewAccountant(accountId, reservation, onDemand, reservationWindow, pricePerSymbol, minNumSymbols, numBins)

ctx := context.Background()
numSymbols := uint32(2000)
numSymbols := uint64(2000)
quorums := []uint8{0, 1}
now := time.Now().UnixNano()
_, err = accountant.AccountBlob(ctx, now, numSymbols, quorums)
Expand All @@ -164,9 +164,9 @@ func TestAccountBlobCallSeries(t *testing.T) {
onDemand := &core.OnDemandPayment{
CumulativePayment: big.NewInt(1000),
}
reservationWindow := uint32(5)
pricePerSymbol := uint32(1)
minNumSymbols := uint32(100)
reservationWindow := uint64(5)
pricePerSymbol := uint64(1)
minNumSymbols := uint64(100)

privateKey1, err := crypto.GenerateKey()
assert.NoError(t, err)
Expand Down Expand Up @@ -217,9 +217,9 @@ func TestAccountBlob_BinRotation(t *testing.T) {
onDemand := &core.OnDemandPayment{
CumulativePayment: big.NewInt(1000),
}
reservationWindow := uint32(1) // Set to 1 second for testing
pricePerSymbol := uint32(1)
minNumSymbols := uint32(100)
reservationWindow := uint64(1) // Set to 1 second for testing
pricePerSymbol := uint64(1)
minNumSymbols := uint64(100)

privateKey1, err := crypto.GenerateKey()
assert.NoError(t, err)
Expand Down Expand Up @@ -258,9 +258,9 @@ func TestConcurrentBinRotationAndAccountBlob(t *testing.T) {
onDemand := &core.OnDemandPayment{
CumulativePayment: big.NewInt(1000),
}
reservationWindow := uint32(1) // Set to 1 second for testing
pricePerSymbol := uint32(1)
minNumSymbols := uint32(100)
reservationWindow := uint64(1) // Set to 1 second for testing
pricePerSymbol := uint64(1)
minNumSymbols := uint64(100)

privateKey1, err := crypto.GenerateKey()
assert.NoError(t, err)
Expand Down Expand Up @@ -301,9 +301,9 @@ func TestAccountBlob_ReservationWithOneOverflow(t *testing.T) {
onDemand := &core.OnDemandPayment{
CumulativePayment: big.NewInt(1000),
}
reservationWindow := uint32(5)
pricePerSymbol := uint32(1)
minNumSymbols := uint32(100)
reservationWindow := uint64(5)
pricePerSymbol := uint64(1)
minNumSymbols := uint64(100)

privateKey1, err := crypto.GenerateKey()
assert.NoError(t, err)
Expand Down Expand Up @@ -348,9 +348,9 @@ func TestAccountBlob_ReservationOverflowReset(t *testing.T) {
onDemand := &core.OnDemandPayment{
CumulativePayment: big.NewInt(1000),
}
reservationWindow := uint32(1) // Set to 1 second for testing
pricePerSymbol := uint32(1)
minNumSymbols := uint32(100)
reservationWindow := uint64(1) // Set to 1 second for testing
pricePerSymbol := uint64(1)
minNumSymbols := uint64(100)

privateKey1, err := crypto.GenerateKey()
assert.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion api/clients/v2/disperser_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func (c *disperserClient) DisperseBlob(
}

symbolLength := encoding.GetBlobLengthPowerOf2(uint(len(data)))
payment, err := c.accountant.AccountBlob(ctx, time.Now().UnixNano(), uint32(symbolLength), quorums)
payment, err := c.accountant.AccountBlob(ctx, time.Now().UnixNano(), uint64(symbolLength), quorums)
if err != nil {
return nil, [32]byte{}, fmt.Errorf("error accounting blob: %w", err)
}
Expand Down
6 changes: 3 additions & 3 deletions api/docs/disperser_v2.html
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions api/docs/disperser_v2.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions api/docs/eigenda-protos.html
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions api/docs/eigenda-protos.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 9 additions & 9 deletions api/grpc/disperser/v2/disperser_v2.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions api/proto/disperser/v2/disperser_v2.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,11 @@ message PaymentGlobalParams {
// Global ratelimit for on-demand dispersals
uint64 global_symbols_per_second = 1;
// Minimum number of symbols accounted for all dispersals
uint32 min_num_symbols = 2;
uint64 min_num_symbols = 2;
// Price charged per symbol for on-demand dispersals
uint32 price_per_symbol = 3;
uint64 price_per_symbol = 3;
// Reservation window for all reservations
uint32 reservation_window = 4;
uint64 reservation_window = 4;
// quorums allowed to make on-demand dispersals
repeated uint32 on_demand_quorum_numbers = 5;
}
Expand Down
Loading
Loading