Skip to content

Commit

Permalink
[RelayMiner] Fix non deleted smt when funds are insufficient to submi…
Browse files Browse the repository at this point in the history
…t 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

<!-- READ & DELETE:
- Documentation changes: only keep this if you're making documentation
changes
- Unit Testing: Remove this if you didn't make code changes
- E2E Testing: Remove this if you didn't make code changes
- See the quickstart guide for instructions:
https://dev.poktroll.com/developer_guide/quickstart
- DevNet E2E Testing: Remove this if you didn't make code changes
- THIS IS VERY EXPENSIVE: only do it after all the reviews are complete.
- Optionally run `make trigger_ci` if you want to re-trigger tests
without any code changes
- If tests fail, try re-running failed tests only using the GitHub UI as
shown
[here](https://github.com/pokt-network/poktroll/assets/1892194/607984e9-0615-4569-9452-4c730190c1d2)
-->

- [ ] **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
  • Loading branch information
red-0ne authored Jan 16, 2025
1 parent a3fb231 commit 0c52ba7
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 7 deletions.
4 changes: 3 additions & 1 deletion Tiltfile
Original file line number Diff line number Diff line change
Expand Up @@ -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"
],
)

Expand Down
33 changes: 28 additions & 5 deletions pkg/relayer/session/claim.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion pkg/relayer/session/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down

0 comments on commit 0c52ba7

Please sign in to comment.