[v2] disperser server payments api #902
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
hopeyen
force-pushed
the
hope/v2-payments
branch
2 times, most recently
from
c0652fa to
4cf7d2c
Compare
hopeyen
force-pushed
the
hope/v2-payments
branch
7 times, most recently
from
89af572 to
e306bc7
Compare
core/auth/v2/authenticator.go
Outdated
| @@ -55,3 +56,36 @@ func (*authenticator) AuthenticateBlobRequest(header *core.BlobHeader) error { | |||
|
|
|||
| return nil | |||
| } | |||
|
|
|||
| func (*authenticator) AuthenticatePaymentStateRequest(request *pb.GetPaymentStateRequest) error { | |||
There was a problem hiding this comment.
This probably belongs in apiserver/server_v2
There was a problem hiding this comment.
isn't this function doing similar authentications?
There was a problem hiding this comment.
I see what you mean. Then let's take the signature bytes as the input vs. the request protobuf in this method, and use this as a library from the api server?
| } | ||
|
|
||
| type BinRecord struct { | ||
| Index uint32 | ||
| Usage uint64 | ||
| } | ||
|
|
||
| func NewAccountant(reservation *core.ActiveReservation, onDemand *core.OnDemandPayment, reservationWindow uint32, pricePerSymbol uint32, minNumSymbols uint32, paymentSigner core.PaymentSigner, numBins uint32) *accountant { | ||
| func NewAccountant(accountID string, reservation *core.ActiveReservation, onDemand *core.OnDemandPayment, reservationWindow uint32, pricePerSymbol uint32, minNumSymbols uint32, numBins uint32) *accountant { |
There was a problem hiding this comment.
Are reservationWindow, pricePerSymbol, minNumSymbols, numBins different for each account?
Aren't these globally available somewhere?
There was a problem hiding this comment.
reservationWindow, pricePerSymbol, minNumSymbols should be set through the contract. I linked a PR in the previous comment about this. numBins can be set by the accountants preferences, doesn't really matter but restricted by a minimum of 3
There was a problem hiding this comment.
in which case does it make sense to account for more than 3 bins?
I'm wondering if we should fix/hardcode this number everywhere vs. making it a variable
There was a problem hiding this comment.
This is just a nice functionality considered for disperser clients who want to track more history in-memory. The data isn't really used and not critical
hopeyen
force-pushed
the
hope/v2-payments
branch
from
1ae3e3c to
8ea4f03
Compare
core/auth/v2/authenticator.go
Outdated
| @@ -55,3 +56,36 @@ func (*authenticator) AuthenticateBlobRequest(header *core.BlobHeader) error { | |||
|
|
|||
| return nil | |||
| } | |||
|
|
|||
| func (*authenticator) AuthenticatePaymentStateRequest(request *pb.GetPaymentStateRequest) error { | |||
There was a problem hiding this comment.
I see what you mean. Then let's take the signature bytes as the input vs. the request protobuf in this method, and use this as a library from the api server?
| @@ -237,3 +237,79 @@ func (s *OffchainStore) GetRelevantOnDemandRecords(ctx context.Context, accountI | |||
|
|
|||
| return prevPayment, nextPayment, nextDataLength, nil | |||
| } | |||
|
|
|||
| func (s *OffchainStore) GetBinUsages(ctx context.Context, accountID string, binIndex uint32) (uint64, uint64, uint64, error) { | |||
| // Fetch the 3 bins start from the current bin | |||
There was a problem hiding this comment.
Does it depend on MinNumBins? Looks like it will always return 3 bin records no matter what MinNumBins is.
| } | ||
|
|
||
| type BinRecord struct { | ||
| Index uint32 | ||
| Usage uint64 | ||
| } | ||
|
|
||
| func NewAccountant(reservation *core.ActiveReservation, onDemand *core.OnDemandPayment, reservationWindow uint32, pricePerSymbol uint32, minNumSymbols uint32, paymentSigner core.PaymentSigner, numBins uint32) *accountant { | ||
| func NewAccountant(accountID string, reservation *core.ActiveReservation, onDemand *core.OnDemandPayment, reservationWindow uint32, pricePerSymbol uint32, minNumSymbols uint32, numBins uint32) *accountant { |
There was a problem hiding this comment.
in which case does it make sense to account for more than 3 bins?
I'm wondering if we should fix/hardcode this number everywhere vs. making it a variable
| @@ -63,6 +66,28 @@ message BlobCommitmentReply { | |||
| common.BlobCommitment blob_commitment = 1; | |||
| } | |||
|
|
|||
| // GetPaymentStateRequest contains parameters to query the payment state of an account. | |||
| message GetPaymentStateRequest { | |||
| string account_id = 1; | |||
There was a problem hiding this comment.
nit: indentation throughout this file
core/meterer/offchain_store.go
Outdated
| @@ -237,3 +240,97 @@ func (s *OffchainStore) GetRelevantOnDemandRecords(ctx context.Context, accountI | |||
|
|
|||
| return prevPayment, nextPayment, nextDataLength, nil | |||
| } | |||
|
|
|||
| func (s *OffchainStore) GetBinRecords(ctx context.Context, accountID string, binIndex uint32) ([3]*pb.BinRecord, error) { | |||
There was a problem hiding this comment.
return [MinNumBins]*pb.BinRecord? (and everywhere in this file)
| return nil, nil | ||
| } | ||
|
|
||
| payment, err := strconv.ParseUint(payments[0]["CumulativePayments"].(*types.AttributeValueMemberN).Value, 10, 64) |
There was a problem hiding this comment.
Are you looking to fix this in a separate PR or this PR? This method returns a bigint but this value is read as uint64, which would be a problem if the value is greater than uint64
Why are these changes needed?
Adding payments to disperser v2 api and server; disperser v2 client is not available yet
Checks