From 7ae3c6038291024ac13587690e17c540b048d760 Mon Sep 17 00:00:00 2001 From: hopeyen Date: Thu, 12 Dec 2024 11:25:00 -0800 Subject: [PATCH] refactor: add unit tests and improve calls --- core/data.go | 29 ++++++++-------- core/data_test.go | 62 +++++++++++++++++++++++++++++++++++ core/meterer/meterer.go | 3 -- core/meterer/onchain_state.go | 25 +++++++++++--- 4 files changed, 99 insertions(+), 20 deletions(-) diff --git a/core/data.go b/core/data.go index 3cd65ba40..367faad32 100644 --- a/core/data.go +++ b/core/data.go @@ -6,7 +6,6 @@ import ( "fmt" "math/big" "strconv" - "time" commonpb "github.com/Layr-Labs/eigenda/api/grpc/common" "github.com/Layr-Labs/eigenda/common" @@ -605,20 +604,24 @@ func ConvertToPaymentMetadata(ph *commonpb.PaymentHeader) *PaymentMetadata { } } -// OperatorInfo contains information about an operator which is stored on the blockchain state, -// corresponding to a particular quorum +// ReservedPayment contains information the onchain state about a reserved payment type ReservedPayment struct { - SymbolsPerSecond uint64 // reserve number of symbols per second - //TODO: we are not using start and end timestamp, add check or remove - StartTimestamp uint64 // Unix timestamp that's valid for basically eternity - EndTimestamp uint64 + // reserve number of symbols per second + SymbolsPerSecond uint64 + // reservation activation timestamp + StartTimestamp uint64 + // reservation expiration timestamp + EndTimestamp uint64 - QuorumNumbers []uint8 // allowed quorums - QuorumSplits []byte // ordered mapping of quorum number to payment split; on-chain validation should ensure split <= 100 + // allowed quorums + QuorumNumbers []uint8 + // ordered mapping of quorum number to payment split; on-chain validation should ensure split <= 100 + QuorumSplits []byte } type OnDemandPayment struct { - CumulativePayment *big.Int // Total amount deposited by the user + // Total amount deposited by the user + CumulativePayment *big.Int } type BlobVersionParameters struct { @@ -627,7 +630,7 @@ type BlobVersionParameters struct { NumChunks uint32 } -func (ar *ReservedPayment) IsActive() bool { - now := uint64(time.Now().Unix()) - return ar.StartTimestamp <= now && ar.EndTimestamp >= now +// IsActive returns true if the reservation is active at the given timestamp +func (ar *ReservedPayment) IsActive(currentTimestamp uint64) bool { + return ar.StartTimestamp <= currentTimestamp && ar.EndTimestamp >= currentTimestamp } diff --git a/core/data_test.go b/core/data_test.go index 84cb5097e..61ffd3724 100644 --- a/core/data_test.go +++ b/core/data_test.go @@ -217,3 +217,65 @@ func TestChunksData(t *testing.T) { assert.EqualError(t, err, "unsupported chunk encoding format: 3") } } + +func TestReservedPayment_IsActive(t *testing.T) { + tests := []struct { + name string + reservedPayment core.ReservedPayment + currentTimestamp uint64 + wantActive bool + }{ + { + name: "active - current time in middle of range", + reservedPayment: core.ReservedPayment{ + StartTimestamp: 100, + EndTimestamp: 200, + }, + currentTimestamp: 150, + wantActive: true, + }, + { + name: "active - current time at start", + reservedPayment: core.ReservedPayment{ + StartTimestamp: 100, + EndTimestamp: 200, + }, + currentTimestamp: 100, + wantActive: true, + }, + { + name: "active - current time at end", + reservedPayment: core.ReservedPayment{ + StartTimestamp: 100, + EndTimestamp: 200, + }, + currentTimestamp: 200, + wantActive: true, + }, + { + name: "inactive - current time before start", + reservedPayment: core.ReservedPayment{ + StartTimestamp: 100, + EndTimestamp: 200, + }, + currentTimestamp: 99, + wantActive: false, + }, + { + name: "inactive - current time after end", + reservedPayment: core.ReservedPayment{ + StartTimestamp: 100, + EndTimestamp: 200, + }, + currentTimestamp: 201, + wantActive: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + isActive := tt.reservedPayment.IsActive(tt.currentTimestamp) + assert.Equal(t, tt.wantActive, isActive) + }) + } +} diff --git a/core/meterer/meterer.go b/core/meterer/meterer.go index e70b51141..2fd9f403b 100644 --- a/core/meterer/meterer.go +++ b/core/meterer/meterer.go @@ -80,9 +80,6 @@ func (m *Meterer) MeterRequest(ctx context.Context, header core.PaymentMetadata, if err != nil { return fmt.Errorf("failed to get active reservation by account: %w", err) } - if !reservation.IsActive() { - return fmt.Errorf("reservation not active") - } if err := m.ServeReservationRequest(ctx, header, reservation, numSymbols, quorumNumbers); err != nil { return fmt.Errorf("invalid reservation: %w", err) } diff --git a/core/meterer/onchain_state.go b/core/meterer/onchain_state.go index bb2ff89ec..221a7b21d 100644 --- a/core/meterer/onchain_state.go +++ b/core/meterer/onchain_state.go @@ -2,8 +2,10 @@ package meterer import ( "context" + "fmt" "sync" "sync/atomic" + "time" "github.com/Layr-Labs/eigenda/core" "github.com/Layr-Labs/eigenda/core/eth" @@ -145,9 +147,18 @@ func (pcs *OnchainPaymentState) RefreshOnchainPaymentState(ctx context.Context, // GetReservedPaymentByAccount returns a pointer to the active reservation for the given account ID; no writes will be made to the reservation func (pcs *OnchainPaymentState) GetReservedPaymentByAccount(ctx context.Context, accountID gethcommon.Address) (*core.ReservedPayment, error) { - pcs.ReservationsLock.RLock() - defer pcs.ReservationsLock.RUnlock() + timestamp := uint64(time.Now().Unix()) + pcs.ReservationsLock.Lock() + defer pcs.ReservationsLock.Unlock() + if reservation, ok := (pcs.ReservedPayments)[accountID]; ok { + if !reservation.IsActive(timestamp) { + // if reservation is expired, remove it from the local state; if it is not activated, we leave the reservation in the local state + if reservation.EndTimestamp < timestamp { + delete(pcs.ReservedPayments, accountID) + } + return nil, fmt.Errorf("reservation not active") + } return reservation, nil } @@ -156,9 +167,15 @@ func (pcs *OnchainPaymentState) GetReservedPaymentByAccount(ctx context.Context, if err != nil { return nil, err } - pcs.ReservationsLock.Lock() + if !res.IsActive(timestamp) { + if res.StartTimestamp > timestamp { + // if reservation is not activated yet, we add it to the local state to reduce future on-chain calls + (pcs.ReservedPayments)[accountID] = res + } + return nil, fmt.Errorf("reservation not active") + } (pcs.ReservedPayments)[accountID] = res - pcs.ReservationsLock.Unlock() + return res, nil }