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.

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

payment failure cases inabox #1035

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+113 −8
Open

payment failure cases inabox #1035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d49ddde
chore: add failure cases for payments
hopeyen Dec 20, 2024
7ba9549
fix: unit tests
hopeyen Dec 20, 2024
9ee1757
fix: integration test
hopeyen Dec 20, 2024
80da8fb
fix: rebase to master
hopeyen Jan 7, 2025
59a70fe
fix: signer update
hopeyen Jan 7, 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
23 changes: 17 additions & 6 deletions contracts/script/SetUpEigenDA.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,20 +174,31 @@ contract SetupEigenDA is EigenDADeployer, EigenLayerUtils {


// Register Reservations for client as the eigenDACommunityMultisig
IPaymentVault.Reservation memory reservation = IPaymentVault.Reservation({
symbolsPerSecond: 452198,
IPaymentVault.Reservation memory reservation1 = IPaymentVault.Reservation({
symbolsPerSecond: 50, // 15k symbols over 5 minute bin interval
startTimestamp: uint64(block.timestamp),
endTimestamp: uint64(block.timestamp + 1000000000),
quorumNumbers: hex"0001",
quorumSplits: hex"3232"
});
address clientAddress = address(0x1aa8226f6d354380dDE75eE6B634875c4203e522);
vm.startBroadcast(msg.sender);
paymentVault.setReservation(clientAddress, reservation);
paymentVault.setReservation(address(0x1aa8226f6d354380dDE75eE6B634875c4203e522), reservation1);

// small reservation
IPaymentVault.Reservation memory reservation2 = IPaymentVault.Reservation({
symbolsPerSecond: 15, // 4500 symbols over 5 minute bin interval
startTimestamp: uint64(block.timestamp),
endTimestamp: uint64(block.timestamp + 1000000000),
quorumNumbers: hex"0001",
quorumSplits: hex"3232"
});
paymentVault.setReservation(address(0x1113cAAD341fa57b14bBCfFa69b591f7032588C0), reservation2);

// Deposit OnDemand
paymentVault.depositOnDemand{value: 0.1 ether}(clientAddress);
vm.stopBroadcast();
paymentVault.depositOnDemand{value: 0.1 ether}(address(0x1aa8226f6d354380dDE75eE6B634875c4203e522));
paymentVault.depositOnDemand{value: 2000 gwei}(address(0xFCAd0B19bB29D4674531d6f115237E16AfCE377c));

vm.stopBroadcast();
// Deposit stakers into EigenLayer and delegate to operators
for (uint256 i = 0; i < stakerPrivateKeys.length; i++) {
vm.startBroadcast(stakerPrivateKeys[i]);
Expand Down
2 changes: 1 addition & 1 deletion disperser/apiserver/disperse_blob_v2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func (s *DispersalServerV2) validateDispersalRequest(ctx context.Context, req *p
return api.NewErrorInvalidArg("payment metadata is required")
}

if len(blobHeader.PaymentMetadata.AccountID) == 0 || blobHeader.PaymentMetadata.ReservationPeriod == 0 || blobHeader.PaymentMetadata.CumulativePayment == nil {
if len(blobHeader.PaymentMetadata.AccountID) == 0 || (blobHeader.PaymentMetadata.ReservationPeriod == 0 && blobHeader.PaymentMetadata.CumulativePayment.Cmp(big.NewInt(0)) == 0) {
return api.NewErrorInvalidArg("invalid payment metadata")
}

Expand Down
2 changes: 1 addition & 1 deletion disperser/apiserver/server_v2_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func TestV2DisperseBlobRequestValidation(t *testing.T) {
PaymentHeader: &pbcommon.PaymentHeader{
AccountId: accountID,
ReservationPeriod: 0,
CumulativePayment: big.NewInt(100).Bytes(),
CumulativePayment: big.NewInt(0).Bytes(),
},
}
blobHeader, err := corev2.BlobHeaderFromProtobuf(invalidReqProto)
Expand Down
94 changes: 94 additions & 0 deletions inabox/tests/integration_v2_payments_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package integration_test
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if these tests could be moved to unit tests? like in accountant_test.go?
Also, should we also test payment failure cases from disperser server if there isn't already? (to cover the case that user isn't running accountatnt)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm I made these tests for the interaction of disperser client and the server, and shows when a blob should be dispersed and when it should fail with respect to payments. I don't think they could be moved to unit test.
Disperser client would build accountant if it wasn't provided with one, so alternatively we could add rogue accountant implementations that makes invalid payment headers

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm I made these tests for the interaction of disperser client and the server

Is the server logic relevant in these tests? It looks like they mostly test the accountant logic (server could have the payment metering disabled and allowed all requests and these tests would still pass)

Disperser client would build accountant if it wasn't provided with one

Yes, but not all requests may originate from the disperser client shipped in this repo. Anyone is free to disperse blobs without running accountant, in which case, API server should enforce correct metering scheme. I was referring to payment failure cases from the API server payment metering, not from the accountant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the server logic relevant in these tests? It looks like they mostly test the accountant logic (server could have the payment metering disabled and allowed all requests and these tests would still pass)

Hmm I understood your concerns. I updated the tests to override the accountant with erroneous payment state so that accountant doesn't account correctly, and focus on testing failures from API server logic. (if payment is disabled, the tests should not pass as the expected failure would instead be nil)

not all requests may originate from the disperser client shipped in this repo. Anyone is free to disperse blobs without running accountant, in which case, API server should enforce correct metering scheme.

by this logic, should we have a set of tests that doesn't use DisperserClient? The existing tests all utilize NewDisperserClient to make API requests


import (
"context"
"crypto/rand"
"time"

"github.com/Layr-Labs/eigenda/api/clients/v2"
auth "github.com/Layr-Labs/eigenda/core/auth/v2"
dispv2 "github.com/Layr-Labs/eigenda/disperser/common/v2"
"github.com/Layr-Labs/eigenda/encoding/utils/codec"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("Inabox v2 Integration", func() {
It("reservation runs out", func() {
ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
defer cancel()

privateKeyHex := "0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdee"
signer := auth.NewLocalBlobRequestSigner(privateKeyHex)

disp, err := clients.NewDisperserClient(&clients.DisperserClientConfig{
Hostname: "localhost",
Port: "32005",
}, signer, nil, nil)
Expect(err).To(BeNil())
Expect(disp).To(Not(BeNil()))

data1 := make([]byte, 992)
_, err = rand.Read(data1)
Expect(err).To(BeNil())
data2 := make([]byte, 123)
_, err = rand.Read(data2)
Expect(err).To(BeNil())

paddedData1 := codec.ConvertByPaddingEmptyByte(data1)
paddedData2 := codec.ConvertByPaddingEmptyByte(data2)

blobStatus1, key1, err := disp.DisperseBlob(ctx, paddedData1, 0, []uint8{0, 1}, 0)
Expect(err).To(BeNil())
Expect(key1).To(Not(BeNil()))
Expect(blobStatus1).To(Not(BeNil()))
Expect(*blobStatus1).To(Equal(dispv2.Queued))

blobStatus2, key2, err := disp.DisperseBlob(ctx, paddedData2, 0, []uint8{0}, 0)
Expect(err).To(BeNil())
Expect(key2).To(Not(BeNil()))
Expect(blobStatus2).To(Not(BeNil()))
Expect(*blobStatus2).To(Equal(dispv2.Queued))

blobStatus3, key3, err := disp.DisperseBlob(ctx, paddedData2, 0, []uint8{0}, 0)
Expect(err.Error()).To(ContainSubstring("neither reservation nor on-demand payment is available"))
Expect(key3).To(Not(BeNil()))
Expect(blobStatus3).To(BeNil())
})

It("ondemand runs out", func() {
ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
defer cancel()

privateKeyHex := "0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
signer := auth.NewLocalBlobRequestSigner(privateKeyHex)

disp, err := clients.NewDisperserClient(&clients.DisperserClientConfig{
Hostname: "localhost",
Port: "32005",
}, signer, nil, nil)
Expect(err).To(BeNil())
Expect(disp).To(Not(BeNil()))

data1 := make([]byte, 992)
_, err = rand.Read(data1)
Expect(err).To(BeNil())
data2 := make([]byte, 123)
_, err = rand.Read(data2)
Expect(err).To(BeNil())

paddedData1 := codec.ConvertByPaddingEmptyByte(data1)
paddedData2 := codec.ConvertByPaddingEmptyByte(data2)

blobStatus1, key1, err := disp.DisperseBlob(ctx, paddedData1, 0, []uint8{0, 1}, 0)
Expect(err).To(BeNil())
Expect(key1).To(Not(BeNil()))
Expect(blobStatus1).To(Not(BeNil()))
Expect(*blobStatus1).To(Equal(dispv2.Queued))

blobStatus2, key2, err := disp.DisperseBlob(ctx, paddedData2, 0, []uint8{0}, 0)
Expect(err.Error()).To(ContainSubstring("neither reservation nor on-demand payment is available"))
Expect(blobStatus2).To(BeNil())
Expect(key2).To(Not(BeNil()))
})
})
Loading