Skip to content

Commit

Permalink
chore: review feedback improvements
Browse files Browse the repository at this point in the history
Co-authored-by: Daniel Olshansky <[email protected]>
Co-authored-by: harry <[email protected]>
  • Loading branch information
3 people committed Dec 22, 2023
1 parent 6e60952 commit edc4e41
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 21 deletions.
6 changes: 4 additions & 2 deletions proto/pocket/supplier/proof.proto
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ option go_package = "github.com/pokt-network/poktroll/x/supplier/types";

message Proof {
string supplier_address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
session.SessionHeader session_header = 2; // the session header of the session that this claim is for
bytes merkle_proof = 3; // the serialized SMT proof
// The session header of the session that this claim is for.
session.SessionHeader session_header = 2;
// The serialized SMST proof from the `#ClosestProof()` method.
bytes closest_merkle_proof = 3;
}

2 changes: 2 additions & 0 deletions x/supplier/client/cli/query_claim.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ func CmdShowClaim() *cobra.Command {
Long: `List a specific claim that the node being queried has access to (if it still exists).
A unique claim can be defined via a session_id that the given supplier participated in.
Claims are pruned, according to protocol parameters, some time after their respective proof has been submitted and any dispute window has elapsed.
This is done to minimize the rate state accumulation by effectively eliminating claims as a long-term factor to persistence requirements.
Example:
$ poktrolld --home=$(POKTROLLD_HOME) q claim show-claims <session_id> <supplier_address> --node $(POCKET_NODE)`,
Expand Down
11 changes: 6 additions & 5 deletions x/supplier/keeper/claim.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper

import (
"encoding/binary"
"fmt"

"github.com/cosmos/cosmos-sdk/store/prefix"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -22,22 +23,22 @@ func (k Keeper) UpsertClaim(ctx sdk.Context, claim types.Claim) {
primaryKey := types.ClaimPrimaryKey(sessionId, claim.SupplierAddress)
primaryStore.Set(primaryKey, claimBz)

logger.Info("upserted claim for supplier %s with primaryKey %s", claim.SupplierAddress, primaryKey)
logger.Info(fmt.Sprintf("upserted claim for supplier %s with primaryKey %s", claim.SupplierAddress, primaryKey))

// Update the address index: supplierAddress -> [ClaimPrimaryKey]
addressStoreIndex := prefix.NewStore(parentStore, types.KeyPrefix(types.ClaimSupplierAddressPrefix))
addressKey := types.ClaimSupplierAddressKey(claim.SupplierAddress, primaryKey)
addressStoreIndex.Set(addressKey, primaryKey)

logger.Info("indexed claim for supplier %s with primaryKey %s", claim.SupplierAddress, primaryKey)
logger.Info(fmt.Sprintf("indexed claim for supplier %s with primaryKey %s", claim.SupplierAddress, primaryKey))

// Update the session end height index: sessionEndHeight -> [ClaimPrimaryKey]
sessionHeightStoreIndex := prefix.NewStore(parentStore, types.KeyPrefix(types.ClaimSessionEndHeightPrefix))
sessionEndBlockHeight := uint64(claim.GetSessionHeader().GetSessionEndBlockHeight())
heightKey := types.ClaimSupplierEndSessionHeightKey(sessionEndBlockHeight, primaryKey)
sessionHeightStoreIndex.Set(heightKey, primaryKey)

logger.Info("indexed claim for supplier %s at session ending height %d", claim.SupplierAddress, sessionEndBlockHeight)
logger.Info(fmt.Sprintf("indexed claim for supplier %s at session ending height %d", claim.SupplierAddress, sessionEndBlockHeight))
}

// RemoveClaim removes a claim from the store
Expand All @@ -51,7 +52,7 @@ func (k Keeper) RemoveClaim(ctx sdk.Context, sessionId, supplierAddr string) {
primaryKey := types.ClaimPrimaryKey(sessionId, supplierAddr)
claim, foundClaim := k.getClaimByPrimaryKey(ctx, primaryKey)
if !foundClaim {
logger.Error("trying to delete non-existent claim with primary key %s for supplier %s and session %s", primaryKey, supplierAddr, sessionId)
logger.Error(fmt.Sprintf("trying to delete non-existent claim with primary key %s for supplier %s and session %s", primaryKey, supplierAddr, sessionId))
return
}

Expand All @@ -68,7 +69,7 @@ func (k Keeper) RemoveClaim(ctx sdk.Context, sessionId, supplierAddr string) {
addressStoreIndex.Delete(addressKey)
sessionHeightStoreIndex.Delete(heightKey)

logger.Info("deleted claim with primary key %s for supplier %s and session %s", primaryKey, supplierAddr, sessionId)
logger.Info(fmt.Sprintf("deleted claim with primary key %s for supplier %s and session %s", primaryKey, supplierAddr, sessionId))
}

// GetClaim returns a Claim given a SessionId & SupplierAddr
Expand Down
15 changes: 8 additions & 7 deletions x/supplier/keeper/msg_server_stake_supplier.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper

import (
"context"
"fmt"

sdkerrors "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -17,10 +18,10 @@ func (k msgServer) StakeSupplier(
ctx := sdk.UnwrapSDKContext(goCtx)

logger := k.Logger(ctx).With("method", "StakeSupplier")
logger.Info("About to stake supplier with msg: %v", msg)
logger.Info(fmt.Sprintf("About to stake supplier with msg: %v", msg))

if err := msg.ValidateBasic(); err != nil {
logger.Error("invalid MsgStakeSupplier: %v", msg)
logger.Error(fmt.Sprintf("invalid MsgStakeSupplier: %v", msg))
return nil, err
}

Expand All @@ -29,11 +30,11 @@ func (k msgServer) StakeSupplier(
var coinsToDelegate sdk.Coin
supplier, isSupplierFound := k.GetSupplier(ctx, msg.Address)
if !isSupplierFound {
logger.Info("Supplier not found. Creating new supplier for address %s", msg.Address)
logger.Info(fmt.Sprintf("Supplier not found. Creating new supplier for address %s", msg.Address))
supplier = k.createSupplier(ctx, msg)
coinsToDelegate = *msg.Stake
} else {
logger.Info("Supplier found. Updating supplier for address %s", msg.Address)
logger.Info(fmt.Sprintf("Supplier found. Updating supplier for address %s", msg.Address))
currSupplierStake := *supplier.Stake
if err = k.updateSupplier(ctx, &supplier, msg); err != nil {
return nil, err
Expand All @@ -44,21 +45,21 @@ func (k msgServer) StakeSupplier(
// Retrieve the address of the supplier
supplierAddress, err := sdk.AccAddressFromBech32(msg.Address)
if err != nil {
logger.Error("could not parse address %s", msg.Address)
logger.Error(fmt.Sprintf("could not parse address %s", msg.Address))
return nil, err
}

// TODO_IMPROVE: Should we avoid making this call if `coinsToDelegate` = 0?
// Send the coins from the supplier to the staked supplier pool
err = k.bankKeeper.DelegateCoinsFromAccountToModule(ctx, supplierAddress, types.ModuleName, []sdk.Coin{coinsToDelegate})
if err != nil {
logger.Error("could not send %v coins from %s to %s module account due to %v", coinsToDelegate, supplierAddress, types.ModuleName, err)
logger.Error(fmt.Sprintf("could not send %v coins from %s to %s module account due to %v", coinsToDelegate, supplierAddress, types.ModuleName, err))
return nil, err
}

// Update the Supplier in the store
k.SetSupplier(ctx, supplier)
logger.Info("Successfully updated supplier stake for supplier: %+v", supplier)
logger.Info(fmt.Sprintf("Successfully updated supplier stake for supplier: %+v", supplier))

return &types.MsgStakeSupplierResponse{}, nil
}
Expand Down
13 changes: 7 additions & 6 deletions x/supplier/keeper/msg_server_unstake_supplier.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper

import (
"context"
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"

Expand All @@ -16,7 +17,7 @@ func (k msgServer) UnstakeSupplier(
ctx := sdk.UnwrapSDKContext(goCtx)

logger := k.Logger(ctx).With("method", "UnstakeSupplier")
logger.Info("About to unstake supplier with msg: %v", msg)
logger.Info(fmt.Sprintf("About to unstake supplier with msg: %v", msg))

if err := msg.ValidateBasic(); err != nil {
return nil, err
Expand All @@ -25,27 +26,27 @@ func (k msgServer) UnstakeSupplier(
// Check if the supplier already exists or not
supplier, isSupplierFound := k.GetSupplier(ctx, msg.Address)
if !isSupplierFound {
logger.Info("Supplier not found. Cannot unstake address %s", msg.Address)
logger.Info(fmt.Sprintf("Supplier not found. Cannot unstake address %s", msg.Address))
return nil, types.ErrSupplierNotFound
}
logger.Info("Supplier found. Unstaking supplier for address %s", msg.Address)
logger.Info(fmt.Sprintf("Supplier found. Unstaking supplier for address %s", msg.Address))

// Retrieve the address of the supplier
supplierAddress, err := sdk.AccAddressFromBech32(msg.Address)
if err != nil {
logger.Error("could not parse address %s", msg.Address)
logger.Error(fmt.Sprintf("could not parse address %s", msg.Address))
return nil, err
}

// Send the coins from the supplier pool back to the supplier
err = k.bankKeeper.UndelegateCoinsFromModuleToAccount(ctx, types.ModuleName, supplierAddress, []sdk.Coin{*supplier.Stake})
if err != nil {
logger.Error("could not send %v coins from %s module to %s account due to %v", supplier.Stake, supplierAddress, types.ModuleName, err)
logger.Error(fmt.Sprintf("could not send %v coins from %s module to %s account due to %v", supplier.Stake, supplierAddress, types.ModuleName, err))
return nil, err
}

// Update the Supplier in the store
k.RemoveSupplier(ctx, supplierAddress.String())
logger.Info("Successfully removed the supplier: %+v", supplier)
logger.Info(fmt.Sprintf("Successfully removed the supplier: %+v", supplier))
return &types.MsgUnstakeSupplierResponse{}, nil
}
2 changes: 1 addition & 1 deletion x/supplier/keeper/proof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func createNProofs(keeper *keeper.Keeper, ctx sdk.Context, n int) []types.Proof
SessionStartBlockHeight: 1,
SessionEndBlockHeight: 1 + sessionkeeper.NumBlocksPerSession,
},
MerkleProof: nil,
ClosestMerkleProof: nil,
}

keeper.UpsertProof(ctx, proofs[i])
Expand Down

0 comments on commit edc4e41

Please sign in to comment.