Skip to content

Commit

Permalink
fix: tests and comments
Browse files Browse the repository at this point in the history
  • Loading branch information
hopeyen committed Dec 12, 2024
1 parent 87e6eec commit 9ee0155
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 15 deletions.
2 changes: 1 addition & 1 deletion api/clients/accountant.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func NewAccountant(accountID string, reservation *core.ReservedPayment, onDemand
// BlobPaymentInfo calculates and records payment information. The accountant
// will attempt to use the active reservation first and check for quorum settings,
// then on-demand if the reservation is not available. The returned values are
// bin index for reservation payments and cumulative payment for on-demand payments,
// reservation period for reservation payments and cumulative payment for on-demand payments,
// and both fields are used to create the payment header and signature
func (a *Accountant) BlobPaymentInfo(ctx context.Context, numSymbols uint64, quorumNumbers []uint8) (uint32, *big.Int, error) {
now := time.Now().Unix()
Expand Down
2 changes: 1 addition & 1 deletion api/docs/disperser_v2.html
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ <h3 id="disperser.v2.GetPaymentStateRequest">GetPaymentStateRequest</h3>
<td><a href="#bytes">bytes</a></td>
<td></td>
<td><p>Signature over the account ID
TODO: sign over a bin index or a nonce to mitigate signature replay attacks </p></td>
TODO: sign over a reservation period or a nonce to mitigate signature replay attacks </p></td>
</tr>

</tbody>
Expand Down
2 changes: 1 addition & 1 deletion api/docs/disperser_v2.md
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ GetPaymentStateRequest contains parameters to query the payment state of an acco
| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| account_id | [string](#string) | | |
| signature | [bytes](#bytes) | | Signature over the account ID TODO: sign over a bin index or a nonce to mitigate signature replay attacks |
| signature | [bytes](#bytes) | | Signature over the account ID TODO: sign over a reservation period or a nonce to mitigate signature replay attacks |



Expand Down
2 changes: 1 addition & 1 deletion api/docs/eigenda-protos.html
Original file line number Diff line number Diff line change
Expand Up @@ -2303,7 +2303,7 @@ <h3 id="disperser.v2.GetPaymentStateRequest">GetPaymentStateRequest</h3>
<td><a href="#bytes">bytes</a></td>
<td></td>
<td><p>Signature over the account ID
TODO: sign over a bin index or a nonce to mitigate signature replay attacks </p></td>
TODO: sign over a reservation period or a nonce to mitigate signature replay attacks </p></td>
</tr>

</tbody>
Expand Down
2 changes: 1 addition & 1 deletion api/docs/eigenda-protos.md
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ GetPaymentStateRequest contains parameters to query the payment state of an acco
| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| account_id | [string](#string) | | |
| signature | [bytes](#bytes) | | Signature over the account ID TODO: sign over a bin index or a nonce to mitigate signature replay attacks |
| signature | [bytes](#bytes) | | Signature over the account ID TODO: sign over a reservation period or a nonce to mitigate signature replay attacks |



Expand Down
2 changes: 1 addition & 1 deletion api/grpc/disperser/v2/disperser_v2.pb.go

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

2 changes: 1 addition & 1 deletion api/proto/disperser/v2/disperser_v2.proto
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ message BlobCommitmentReply {
message GetPaymentStateRequest {
string account_id = 1;
// Signature over the account ID
// TODO: sign over a bin index or a nonce to mitigate signature replay attacks
// TODO: sign over a reservation period or a nonce to mitigate signature replay attacks
bytes signature = 2;
}

Expand Down
8 changes: 4 additions & 4 deletions core/meterer/meterer.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (m *Meterer) ServeReservationRequest(ctx context.Context, header core.Payme
return fmt.Errorf("invalid quorum for reservation: %w", err)
}
if !m.ValidateReservationPeriod(header, reservation) {
return fmt.Errorf("invalid bin index for reservation")
return fmt.Errorf("invalid reservation period for reservation")
}

// Update bin usage atomically and check against reservation's data rate as the bin limit
Expand Down Expand Up @@ -135,12 +135,12 @@ func (m *Meterer) ValidateQuorum(headerQuorums []uint8, allowedQuorums []uint8)
return nil
}

// ValidateReservationPeriod checks if the provided bin index is valid
// ValidateReservationPeriod checks if the provided reservation period is valid
func (m *Meterer) ValidateReservationPeriod(header core.PaymentMetadata, reservation *core.ReservedPayment) bool {
now := uint64(time.Now().Unix())
reservationWindow := m.ChainPaymentState.GetReservationWindow()
currentReservationPeriod := GetReservationPeriod(now, reservationWindow)
// Valid bin indexes are either the current bin or the previous bin
// Valid reservation periodes are either the current bin or the previous bin
if (header.ReservationPeriod != currentReservationPeriod && header.ReservationPeriod != (currentReservationPeriod-1)) || (GetReservationPeriod(reservation.StartTimestamp, reservationWindow) > header.ReservationPeriod || header.ReservationPeriod > GetReservationPeriod(reservation.EndTimestamp, reservationWindow)) {
return false
}
Expand Down Expand Up @@ -173,7 +173,7 @@ func (m *Meterer) IncrementBinUsage(ctx context.Context, header core.PaymentMeta
return fmt.Errorf("overflow usage exceeds bin limit")
}

// GetReservationPeriod returns the current bin index by chunking time by the bin interval;
// GetReservationPeriod returns the current reservation period by chunking time by the bin interval;
// bin interval used by the disperser should be public information
func GetReservationPeriod(timestamp uint64, binInterval uint32) uint32 {
return uint32(timestamp) / binInterval
Expand Down
25 changes: 21 additions & 4 deletions core/meterer/meterer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ var (
accountID2 gethcommon.Address
account2Reservations *core.ReservedPayment
account2OnDemandPayments *core.OnDemandPayment
accountID3 gethcommon.Address
account3Reservations *core.ReservedPayment
mt *meterer.Meterer

deployLocalStack bool
Expand Down Expand Up @@ -100,6 +102,11 @@ func setup(_ *testing.M) {
teardown()
panic("failed to generate private key")
}
privateKey3, err := crypto.GenerateKey()
if err != nil {
teardown()
panic("failed to generate private key")
}

logger = logging.NewNoopLogger()
config := meterer.Config{
Expand All @@ -126,8 +133,10 @@ func setup(_ *testing.M) {
now := uint64(time.Now().Unix())
accountID1 = crypto.PubkeyToAddress(privateKey1.PublicKey)
accountID2 = crypto.PubkeyToAddress(privateKey2.PublicKey)
account1Reservations = &core.ReservedPayment{SymbolsPerSecond: 100, StartTimestamp: now + 1200, EndTimestamp: now + 1800, QuorumSplits: []byte{50, 50}, QuorumNumbers: []uint8{0, 1}}
accountID3 = crypto.PubkeyToAddress(privateKey3.PublicKey)
account1Reservations = &core.ReservedPayment{SymbolsPerSecond: 100, StartTimestamp: now - 120, EndTimestamp: now + 180, QuorumSplits: []byte{50, 50}, QuorumNumbers: []uint8{0, 1}}
account2Reservations = &core.ReservedPayment{SymbolsPerSecond: 200, StartTimestamp: now - 120, EndTimestamp: now + 180, QuorumSplits: []byte{30, 70}, QuorumNumbers: []uint8{0, 1}}
account3Reservations = &core.ReservedPayment{SymbolsPerSecond: 200, StartTimestamp: now + 120, EndTimestamp: now + 180, QuorumSplits: []byte{30, 70}, QuorumNumbers: []uint8{0, 1}}
account1OnDemandPayments = &core.OnDemandPayment{CumulativePayment: big.NewInt(3864)}
account2OnDemandPayments = &core.OnDemandPayment{CumulativePayment: big.NewInt(2000)}

Expand Down Expand Up @@ -183,6 +192,9 @@ func TestMetererReservations(t *testing.T) {
paymentChainState.On("GetReservedPaymentByAccount", testifymock.Anything, testifymock.MatchedBy(func(account gethcommon.Address) bool {
return account == accountID2
})).Return(account2Reservations, nil)
paymentChainState.On("GetReservedPaymentByAccount", testifymock.Anything, testifymock.MatchedBy(func(account gethcommon.Address) bool {
return account == accountID3
})).Return(account3Reservations, nil)
paymentChainState.On("GetReservedPaymentByAccount", testifymock.Anything, testifymock.Anything).Return(&core.ReservedPayment{}, fmt.Errorf("reservation not found"))

// test invalid quorom ID
Expand All @@ -209,10 +221,15 @@ func TestMetererReservations(t *testing.T) {
err = mt.MeterRequest(ctx, *header, 1000, []uint8{0, 1, 2})
assert.ErrorContains(t, err, "failed to get active reservation by account: reservation not found")

// test invalid bin index
header = createPaymentHeader(reservationPeriod, big.NewInt(0), accountID1)
// test inactive reservation
header = createPaymentHeader(reservationPeriod, big.NewInt(0), accountID3)
err = mt.MeterRequest(ctx, *header, 1000, []uint8{0})
assert.ErrorContains(t, err, "reservation not active")

// test invalid reservation period
header = createPaymentHeader(reservationPeriod-3, big.NewInt(0), accountID1)
err = mt.MeterRequest(ctx, *header, 2000, quoromNumbers)
assert.ErrorContains(t, err, "invalid bin index for reservation")
assert.ErrorContains(t, err, "invalid reservation period for reservation")

// test bin usage metering
symbolLength := uint(20)
Expand Down

0 comments on commit 9ee0155

Please sign in to comment.