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

Payout to committers in case of data request timeout #461

Merged
merged 3 commits into from
Jan 15, 2025
Merged

Payout to committers in case of data request timeout #461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
15a9544
feat(x/tally): fixed payout to committers when data request expires
hacheigriega Jan 10, 2025
2665824
chore(x/tally): skip committer payout calc if there is no commit
hacheigriega Jan 11, 2025
7cc82fe
fix(x/tally): multiply commit gas cost with request gas price
hacheigriega Jan 14, 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
9 changes: 7 additions & 2 deletions integration/integration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func NewIntegrationApp(
logger log.Logger,
keys map[string]*storetypes.KVStoreKey,
appCodec codec.Codec,
router *baseapp.MsgServiceRouter,
modules map[string]appmodule.AppModule,
) *IntegationApp {
db := dbm.NewMemDB()
Expand Down Expand Up @@ -78,8 +79,12 @@ func NewIntegrationApp(
return moduleManager.EndBlock(sdkCtx)
})

router := baseapp.NewMsgServiceRouter()
router.SetInterfaceRegistry(interfaceRegistry)
configurator := module.NewConfigurator(appCodec, router, bApp.GRPCQueryRouter())
err := moduleManager.RegisterServices(configurator)
if err != nil {
panic(err)
}
bApp.SetMsgServiceRouter(router)

if keys[consensusparamtypes.StoreKey] != nil {
Expand All @@ -104,7 +109,7 @@ func NewIntegrationApp(
}
}

_, err := bApp.Commit()
_, err = bApp.Commit()
if err != nil {
panic(err)
}
Expand Down
3 changes: 3 additions & 0 deletions proto/sedachain/tally/v1/tally.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,7 @@ message Params {
// FilterGasCostMultiplierStdDev is the gas cost multiplier for a filter type
// stddev.
uint64 filter_gas_cost_multiplier_stddev = 4;
// GasCostCommitment is the gas cost for a commitment corresponding to an
// expired data request.
uint64 gas_cost_commitment = 5;
}
6 changes: 4 additions & 2 deletions x/batching/keeper/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/ethereum/go-ethereum/crypto"
"github.com/sedaprotocol/seda-chain/app/utils"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/codec"
addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/runtime"
Expand Down Expand Up @@ -178,14 +179,15 @@ func initFixture(tb testing.TB) *fixture {
require.NoError(tb, err)

// x/wasm
router := baseapp.NewMsgServiceRouter()
wasmKeeper := wasmkeeper.NewKeeper(
cdc,
runtime.NewKVStoreService(keys[wasmtypes.StoreKey]),
accountKeeper,
bankKeeper,
stakingKeeper,
nil, nil, nil, nil,
nil, nil, nil, nil,
nil, nil, router, nil,
tempDir,
wasmtypes.DefaultWasmConfig(),
wasmCapabilities,
Expand Down Expand Up @@ -249,7 +251,7 @@ func initFixture(tb testing.TB) *fixture {
pubKeyModule := pubkey.NewAppModule(cdc, pubKeyKeeper)
batchingModule := batching.NewAppModule(cdc, batchingKeeper)

integrationApp := integration.NewIntegrationApp(ctx, logger, keys, cdc, map[string]appmodule.AppModule{
integrationApp := integration.NewIntegrationApp(ctx, logger, keys, cdc, router, map[string]appmodule.AppModule{
authtypes.ModuleName: authModule,
banktypes.ModuleName: bankModule,
sdkstakingtypes.ModuleName: stakingModule,
Expand Down
25 changes: 11 additions & 14 deletions x/pubkey/keeper/endblock_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/codec"
addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
Expand Down Expand Up @@ -48,7 +49,6 @@ import (
"github.com/sedaprotocol/seda-chain/x/pubkey/types"
"github.com/sedaprotocol/seda-chain/x/staking"
stakingkeeper "github.com/sedaprotocol/seda-chain/x/staking/keeper"
stakingtypes "github.com/sedaprotocol/seda-chain/x/staking/types"
"github.com/sedaprotocol/seda-chain/x/vesting"
)

Expand Down Expand Up @@ -157,19 +157,16 @@ func initFixture(tb testing.TB) *fixture {
stakingModule := staking.NewAppModule(cdc, stakingKeeper, accountKeeper, bankKeeper, pubKeyKeeper)
pubkeyModule := pubkey.NewAppModule(cdc, pubKeyKeeper)

integrationApp := integration.NewIntegrationApp(newCtx, logger, keys, cdc, map[string]appmodule.AppModule{
authtypes.ModuleName: authModule,
banktypes.ModuleName: bankModule,
sdkstakingtypes.ModuleName: stakingModule,
types.ModuleName: pubkeyModule,
})

types.RegisterMsgServer(integrationApp.MsgServiceRouter(), keeper.NewMsgServerImpl(pubKeyKeeper))

sdkStakingMsgServer := sdkstakingkeeper.NewMsgServerImpl(sdkStakingKeeper)
stakingMsgServer := stakingkeeper.NewMsgServerImpl(sdkStakingMsgServer, stakingKeeper)
sdkstakingtypes.RegisterMsgServer(integrationApp.MsgServiceRouter(), stakingMsgServer)
stakingtypes.RegisterMsgServer(integrationApp.MsgServiceRouter(), stakingMsgServer)
integrationApp := integration.NewIntegrationApp(
newCtx, logger, keys, cdc,
baseapp.NewMsgServiceRouter(),
map[string]appmodule.AppModule{
authtypes.ModuleName: authModule,
banktypes.ModuleName: bankModule,
sdkstakingtypes.ModuleName: stakingModule,
types.ModuleName: pubkeyModule,
},
)

return &fixture{
IntegationApp: integrationApp,
Expand Down
60 changes: 30 additions & 30 deletions x/tally/keeper/endblock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"strconv"
"strings"

"cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/sedaprotocol/seda-wasm-vm/tallyvm/v2"
Expand Down Expand Up @@ -74,8 +75,8 @@ func (k Keeper) ProcessTallies(ctx sdk.Context, coreContract sdk.AccAddress) err

// Loop through the list to apply filter, execute tally, and post
// execution result.
processedReqs := make(map[string]types.DistributionMessages)
tallyResults := make([]TallyResult, len(tallyList))
sudoMsgs := make([]types.SudoRemoveDataRequest, len(tallyList))
dataResults := make([]batchingtypes.DataResult, len(tallyList))
for i, req := range tallyList {
dataResults[i] = batchingtypes.DataResult{
Expand All @@ -90,15 +91,32 @@ func (k Keeper) ProcessTallies(ctx sdk.Context, coreContract sdk.AccAddress) err
SedaPayload: req.SedaPayload,
}

gasPriceInt, ok := math.NewIntFromString(req.GasPrice)
if !ok {
return fmt.Errorf("invalid gas price: %s", req.GasPrice) // TODO improve error handling
}

var distMsgs types.DistributionMessages
var err error
switch {
case len(req.Commits) == 0 || len(req.Commits) < int(req.ReplicationFactor):
dataResults[i].Result = []byte(fmt.Sprintf("need %d commits; received %d", req.ReplicationFactor, len(req.Commits)))
dataResults[i].ExitCode = TallyExitCodeNotEnoughCommits
k.Logger(ctx).Info("data request's number of commits did not meet replication factor", "request_id", req.ID)

distMsgs, err = k.CalculateCommitterPayouts(ctx, req, gasPriceInt)
if err != nil {
return err
}
case len(req.Reveals) == 0 || len(req.Reveals) < int(req.ReplicationFactor):
dataResults[i].Result = []byte(fmt.Sprintf("need %d reveals; received %d", req.ReplicationFactor, len(req.Reveals)))
dataResults[i].ExitCode = TallyExitCodeNotEnoughReveals
k.Logger(ctx).Info("data request's number of reveals did not meet replication factor", "request_id", req.ID)

distMsgs, err = k.CalculateCommitterPayouts(ctx, req, gasPriceInt)
if err != nil {
return err
}
default:
tallyResults[i] = k.FilterAndTally(ctx, req)
dataResults[i].Result = tallyResults[i].result
Expand All @@ -109,31 +127,27 @@ func (k Keeper) ProcessTallies(ctx sdk.Context, coreContract sdk.AccAddress) err

k.Logger(ctx).Info("completed tally", "request_id", req.ID)
k.Logger(ctx).Debug("tally result", "request_id", req.ID, "tally_result", tallyResults[i])

// TODO
distMsgs = types.DistributionMessages{
Messages: []types.DistributionMessage{},
RefundType: types.DistributionTypeNoConsensus,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should add one for insufficient commits/reveals :x

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure what you mean here? :) The switch statement covers those cases right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Your switch case does! I was thinking about the RefundType message. It would be more specific for the end user than saying No Consensus.

Copy link
Contributor

Choose a reason for hiding this comment

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

Idk what do you think @Thomasvdam?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes I find the current list of distribution types to be confusing.

const (
	DistributionTypeTallyReward     DistributionType = "tally_reward"
	DistributionTypeExecutorReward  DistributionType = "executor_reward"
	DistributionTypeTimedOut        DistributionType = "timed_out"
	DistributionTypeNoConsensus     DistributionType = "no_consensus"
	DistributionTypeRemainderRefund DistributionType = "remainder_refund"
)

We have to add executor reward can be inferred from the distribution kind. Also, time out and no consensus are outcomes of tally, not distribution types, right? We also have to add distribution kinds for slashing and data provider payout, but I'm not sure what distribution types they would be.

Copy link
Contributor

Choose a reason for hiding this comment

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

No consensus and time out exists, so the dr poster knows why they got a full refund. At least that's the logic I'm going by.

Copy link
Contributor

@gluax gluax Jan 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

As for data provider payout I would assume it would be a DistributionSend message, while I thought we weren't doing any slashing to executors?

Oopa nevermind new Distribution makes sense to match Executor reward.

}
}

processedReqs[req.ID] = distMsgs
dataResults[i].Id, err = dataResults[i].TryHash()
if err != nil {
return err
}
sudoMsgs[i] = types.SudoRemoveDataRequest{ID: req.ID}
}

// Notify the Core Contract of tally completion.
msg, err := json.Marshal(struct {
SudoRemoveDataRequests struct {
Requests []types.SudoRemoveDataRequest `json:"requests"`
} `json:"remove_data_requests"`
}{
SudoRemoveDataRequests: struct {
Requests []types.SudoRemoveDataRequest `json:"requests"`
}{
Requests: sudoMsgs,
},
})
msg, err := types.MarshalSudoRemoveDataRequests(processedReqs)
if err != nil {
return err
}
postRes, err := k.wasmKeeper.Sudo(ctx, coreContract, msg)
_, err = k.wasmKeeper.Sudo(ctx, coreContract, msg)
if err != nil {
return err
}
Expand All @@ -144,14 +158,8 @@ func (k Keeper) ProcessTallies(ctx sdk.Context, coreContract sdk.AccAddress) err
if err != nil {
return err
}
}

for i := range sudoMsgs {
k.Logger(ctx).Info(
"tally flow completed",
"request_id", dataResults[i].DrId,
"post_result", postRes,
)
k.Logger(ctx).Info("tally flow completed", "request_id", dataResults[i].DrId)
ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeTallyCompletion,
Expand All @@ -167,6 +175,7 @@ func (k Keeper) ProcessTallies(ctx sdk.Context, coreContract sdk.AccAddress) err
),
)
}

return nil
}

Expand Down Expand Up @@ -243,12 +252,3 @@ func (k Keeper) logErrAndRet(ctx sdk.Context, baseErr, registeredErr error, req
k.Logger(ctx).Debug(baseErr.Error(), "request_id", req.ID, "error", registeredErr)
return registeredErr
}

// TODO: This will become more complex when we introduce incentives.
func calculateExecGasUsed(reveals []types.RevealBody) uint64 {
var execGasUsed uint64
for _, reveal := range reveals {
execGasUsed += reveal.GasUsed
}
return execGasUsed
}
100 changes: 90 additions & 10 deletions x/tally/keeper/endblock_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,105 @@ import (

"github.com/sedaprotocol/seda-wasm-vm/tallyvm/v2"

"github.com/sedaprotocol/seda-chain/x/tally/keeper"
"github.com/sedaprotocol/seda-chain/x/tally/keeper/testdata"
"github.com/sedaprotocol/seda-chain/x/tally/types"
)

func TestProcessTallies(t *testing.T) {
func TestEndBlock(t *testing.T) {
f := initFixture(t)

drID := f.commitRevealDataRequest(t, "c2VkYXByb3RvY29s")
tests := []struct {
name string
memo string
replicationFactor int
numCommits int
numReveals int
timeout bool
expExitCode uint32
}{
{
name: "full single commit-reveal",
memo: "YzJWamRYSmxaR0YwWVE9PQ==",
replicationFactor: 1,
numCommits: 1,
numReveals: 1,
timeout: false,
expExitCode: 0,
},
{
name: "full 5 commit-reveals",
memo: "ZnVsbCA1IGNvbW1pdC1yZXZlYWxz",
replicationFactor: 5,
numCommits: 5,
numReveals: 5,
timeout: false,
expExitCode: 0,
},
{
name: "commit timeout",
memo: "Y29tbWl0IHRpbWVvdXQ=",
replicationFactor: 2,
numCommits: 0,
numReveals: 0,
timeout: true,
expExitCode: keeper.TallyExitCodeNotEnoughCommits,
},
{
name: "commit timeout with 1 commit",
memo: "Y29tbWl0IHRpbWVvdXQgd2l0aCAxIGNvbW1pdA==",
replicationFactor: 2,
numCommits: 1,
numReveals: 0,
timeout: true,
expExitCode: keeper.TallyExitCodeNotEnoughCommits,
},
{
name: "commit timeout with 2 commits",
memo: "Y29tbWl0IHRpbWVvdXQgd2l0aCAyIGNvbW1pdHM=",
replicationFactor: 2,
numCommits: 1,
numReveals: 0,
timeout: true,
expExitCode: keeper.TallyExitCodeNotEnoughCommits,
},
{
name: "reveal timeout",
memo: "cmV2ZWFsIHRpbWVvdXQ=",
replicationFactor: 2,
numCommits: 2,
numReveals: 0,
timeout: true,
expExitCode: keeper.TallyExitCodeNotEnoughReveals,
},
{
name: "reveal timeout with 2 reveals",
memo: "cmV2ZWFsIHRpbWVvdXQgd2l0aCAyIHJldmVhbHM=",
replicationFactor: 3,
numCommits: 3,
numReveals: 2,
timeout: true,
expExitCode: keeper.TallyExitCodeNotEnoughReveals,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
drID := f.commitRevealDataRequest(t, tt.memo, tt.replicationFactor, tt.numCommits, tt.numReveals, tt.timeout)

err := f.tallyKeeper.ProcessTallies(f.Context(), f.coreContractAddr)
require.NoError(t, err)
err := f.tallyKeeper.EndBlock(f.Context())
require.NoError(t, err)

dataResult, err := f.batchingKeeper.GetLatestDataResult(f.Context(), drID)
require.NoError(t, err)
require.Equal(t, uint32(0), dataResult.ExitCode)
dataResult, err := f.batchingKeeper.GetLatestDataResult(f.Context(), drID)
require.NoError(t, err)
require.Equal(t, tt.expExitCode, dataResult.ExitCode)

dataResults, err := f.batchingKeeper.GetDataResults(f.Context(), false)
require.NoError(t, err)
require.Contains(t, dataResults, *dataResult)
hacheigriega marked this conversation as resolved.
Show resolved Hide resolved

dataResults, err := f.batchingKeeper.GetDataResults(f.Context(), false)
require.NoError(t, err)
require.Equal(t, *dataResult, dataResults[0])
// TODO: Check before and after balances
})
}
}

// TestTallyVM tests tally VM using a sample tally wasm that performs
Expand Down
Loading
Loading