From 0c52ba75dd8516f297634e9549d46fd604a1ebd5 Mon Sep 17 00:00:00 2001 From: Redouane Lakrache Date: Thu, 16 Jan 2025 03:40:02 +0100 Subject: [PATCH] [RelayMiner] Fix non deleted smt when funds are insufficient to submit C&P (#1026) ## Summary This pull request includes changes to `pkg/relayer/session/claim.go` to 1. Delete SMTs when funds are insufficient. 2. Account for the gas cost of creating claims and submitting proofs. ## Issue When the SupplierOperator lacks sufficient funds to process claim and proof submissions, the corresponding SMTs are not deleted from the file system. This issue becomes more severe at scale, especially when the RelayMiner is handling multiple claims from various applications. ![image](https://github.com/user-attachments/assets/024cb8c2-9190-4f60-9c81-da66c5a2dd0a) ## Type of change Select one or more from the following: - [ ] New feature, functionality or library - [ ] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [x] Bug fix - [ ] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [x] **Unit Tests**: `make go_develop_and_test` - [x] **LocalNet E2E Tests**: `make test_e2e` - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [x] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --- Tiltfile | 4 +++- pkg/relayer/session/claim.go | 33 ++++++++++++++++++++++++----- pkg/relayer/session/session_test.go | 5 ++++- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/Tiltfile b/Tiltfile index b765db324..7a9af482d 100644 --- a/Tiltfile +++ b/Tiltfile @@ -363,7 +363,9 @@ for x in range(localnet_config["path_gateways"]["count"]): # ], # TODO_IMPROVE(@okdas): Add port forwards to grafana, pprof, like the other resources port_forwards=[ - str(2999 + actor_number) + ":3000" + # See PATH for the default port used by the gateway. As of PR #1026, it is :3069. + # https://github.com/buildwithgrove/path/blob/main/config/router.go + str(2999 + actor_number) + ":3069" ], ) diff --git a/pkg/relayer/session/claim.go b/pkg/relayer/session/claim.go index e598ab174..4d09fb2a7 100644 --- a/pkg/relayer/session/claim.go +++ b/pkg/relayer/session/claim.go @@ -5,8 +5,10 @@ import ( "fmt" "slices" + sdktypes "github.com/cosmos/cosmos-sdk/types" "github.com/pokt-network/smt" + "github.com/pokt-network/poktroll/app/volatile" "github.com/pokt-network/poktroll/pkg/client" "github.com/pokt-network/poktroll/pkg/either" "github.com/pokt-network/poktroll/pkg/observable" @@ -18,6 +20,13 @@ import ( sharedtypes "github.com/pokt-network/poktroll/x/shared/types" ) +// The cumulative fees of creating a single claim, followed by submitting a single proof. +// The value was obtained empirically by observing logs during load testing and observing +// the claim & proof lifecycle. +// The gas price at the time of observance was 0.01uPOKT. +// The value is subject to change as the network parameters change. +var ClamAndProofGasCost = sdktypes.NewInt64Coin(volatile.DenomuPOKT, 50000) + // createClaims maps over the sessionsToClaimObs observable. For each claim batch, it: // 1. Calculates the earliest block height at which it is safe to CreateClaims // 2. Waits for said block and creates the claims onchain @@ -260,7 +269,10 @@ func (rs *relayerSessionsManager) payableProofsSessionTrees( if err != nil { return nil, err } - proofSubmissionFeeCoin := proofParams.GetProofSubmissionFee() + + // Account for the gas cost of creating a claim and submitting a proof in addition + // to the ProofSubmissionFee. + claimAndProofSubmissionCost := proofParams.GetProofSubmissionFee().Add(ClamAndProofGasCost) supplierOperatorBalanceCoin, err := rs.bankQueryClient.GetBalance( ctx, @@ -301,19 +313,30 @@ func (rs *relayerSessionsManager) payableProofsSessionTrees( for _, sessionTree := range sessionTrees { // If the supplier operator can afford to claim the session, add it to the // claimableSessionTrees slice. - if supplierOperatorBalanceCoin.IsGTE(*proofSubmissionFeeCoin) { + supplierCanAffordClaimAndProofFees := supplierOperatorBalanceCoin.IsGTE(claimAndProofSubmissionCost) + if supplierCanAffordClaimAndProofFees { claimableSessionTrees = append(claimableSessionTrees, sessionTree) - newSupplierOperatorBalanceCoin := supplierOperatorBalanceCoin.Sub(*proofSubmissionFeeCoin) + newSupplierOperatorBalanceCoin := supplierOperatorBalanceCoin.Sub(claimAndProofSubmissionCost) supplierOperatorBalanceCoin = &newSupplierOperatorBalanceCoin continue } + // At this point supplierCanAffordClaimAndProofFees is false. + // Delete the session tree from the relayer sessions and the KVStore since + // it won't be claimed due to insufficient funds. + rs.removeFromRelayerSessions(sessionTree) + if err := sessionTree.Delete(); err != nil { + logger.With( + "session_id", sessionTree.GetSessionHeader().GetSessionId(), + ).Error().Err(err).Msg("failed to delete session tree") + } + // Log a warning of any session that the supplier operator cannot afford to claim. logger.With( "session_id", sessionTree.GetSessionHeader().GetSessionId(), "supplier_operator_balance", supplierOperatorBalanceCoin, - "proof_submission_fee", proofSubmissionFeeCoin, - ).Warn().Msg("supplier operator cannot afford to submit proof for claim, skipping") + "proof_submission_fee", claimAndProofSubmissionCost, + ).Warn().Msg("supplier operator cannot afford to submit proof for claim, deleting session tree") } if len(claimableSessionTrees) < len(sessionTrees) { diff --git a/pkg/relayer/session/session_test.go b/pkg/relayer/session/session_test.go index 5b5757040..a9017bcad 100644 --- a/pkg/relayer/session/session_test.go +++ b/pkg/relayer/session/session_test.go @@ -207,8 +207,11 @@ func TestRelayerSessionsManager_InsufficientBalanceForProofSubmission(t *testing supplierOperatorAddress := sample.AccAddress() supplierOperatorAccAddress := sdktypes.MustAccAddressFromBech32(supplierOperatorAddress) + + proofSubmissionFee := prooftypes.DefaultParams().ProofSubmissionFee.Amount.Int64() + claimAndProofGasCost := session.ClamAndProofGasCost.Amount.Int64() // Set the supplier operator balance to be able to submit only a single proof. - supplierOperatorBalance := prooftypes.DefaultParams().ProofSubmissionFee.Amount.Int64() + 1 + supplierOperatorBalance := proofSubmissionFee + claimAndProofGasCost + 1 supplierClientMock.EXPECT(). OperatorAddress(). Return(&supplierOperatorAccAddress).