From cc04dc742b831d1e8228b95152d03ecaea54ca84 Mon Sep 17 00:00:00 2001 From: hacheigriega Date: Fri, 20 Dec 2024 15:33:19 +0900 Subject: [PATCH 1/3] refactor(x/tally): reorder tally process and add a filter constructor --- x/tally/keeper/endblock.go | 69 +++++++++++++++++------------- x/tally/keeper/filter.go | 64 ++++++++++++++------------- x/tally/keeper/filter_fuzz_test.go | 7 ++- x/tally/keeper/filter_test.go | 14 +++--- x/tally/types/codec_test.go | 2 +- x/tally/types/filters.go | 65 +++++++++++++++++++--------- 6 files changed, 135 insertions(+), 86 deletions(-) diff --git a/x/tally/keeper/endblock.go b/x/tally/keeper/endblock.go index 19140501..1fe1f097 100644 --- a/x/tally/keeper/endblock.go +++ b/x/tally/keeper/endblock.go @@ -209,44 +209,57 @@ func (k Keeper) FilterAndTally(ctx sdk.Context, req types.Request) (TallyResult, sort.Strings(reveals[i].ProxyPubKeys) } - result.execGasUsed = calculateExecGasUsed(reveals) - - filter, err := base64.StdEncoding.DecodeString(req.ConsensusFilter) - if err != nil { - return result, k.logErrAndRet(ctx, err, types.ErrDecodingConsensusFilter, req) - } - // Convert base64-encoded payback address to hex encoding that - // the tally VM expects. - decodedBytes, err := base64.StdEncoding.DecodeString(req.PaybackAddress) + // Phase I: Filtering + filter, err := k.BuildFilter(ctx, req.ConsensusFilter, req.ReplicationFactor) if err != nil { - return result, k.logErrAndRet(ctx, err, types.ErrDecodingPaybackAddress, req) + return result, err } - paybackAddrHex := hex.EncodeToString(decodedBytes) - - filterResult, err := k.ApplyFilter(ctx, filter, reveals, req.ReplicationFactor) + filterResult, err := ApplyFilter(filter, reveals) result.consensus = filterResult.Consensus result.proxyPubKeys = filterResult.ProxyPubKeys if err != nil { return result, k.logErrAndRet(ctx, err, types.ErrApplyingFilter, req) } + // Phase II: Tally Program Execution + vmRes, err := k.ExecuteTallyProgram(ctx, req, filterResult, reveals) + if err != nil { + return TallyResult{}, err + } + result.stdout = vmRes.Stdout + result.stderr = vmRes.Stderr + result.result = vmRes.Result + result.exitInfo = vmRes.ExitInfo + result.tallyGasUsed = vmRes.GasUsed + filterResult.GasUsed + + // Phase III: Calculate Payouts + result.execGasUsed = calculateExecGasUsed(reveals) + + return result, nil +} + +func (k Keeper) ExecuteTallyProgram(ctx sdk.Context, req types.Request, filterResult FilterResult, reveals []types.RevealBody) (tallyvm.VmResult, error) { tallyProgram, err := k.wasmStorageKeeper.GetOracleProgram(ctx, req.TallyProgramID) if err != nil { - return result, k.logErrAndRet(ctx, err, types.ErrFindingTallyProgram, req) + return tallyvm.VmResult{}, k.logErrAndRet(ctx, err, types.ErrFindingTallyProgram, req) } tallyInputs, err := base64.StdEncoding.DecodeString(req.TallyInputs) if err != nil { - return result, k.logErrAndRet(ctx, err, types.ErrDecodingTallyInputs, req) + return tallyvm.VmResult{}, k.logErrAndRet(ctx, err, types.ErrDecodingTallyInputs, req) } - args, err := tallyVMArg(tallyInputs, reveals, filterResult.Outliers) + // Convert base64-encoded payback address to hex encoding that + // the tally VM expects. + decodedBytes, err := base64.StdEncoding.DecodeString(req.PaybackAddress) if err != nil { - return result, k.logErrAndRet(ctx, err, types.ErrConstructingTallyVMArgs, req) + return tallyvm.VmResult{}, k.logErrAndRet(ctx, err, types.ErrDecodingPaybackAddress, req) } + paybackAddrHex := hex.EncodeToString(decodedBytes) + // Adjust gas limit based on the gas used by the filter. maxGasLimit, err := k.GetMaxTallyGasLimit(ctx) if err != nil { - return result, k.logErrAndRet(ctx, err, types.ErrGettingMaxTallyGasLimit, req) + return tallyvm.VmResult{}, k.logErrAndRet(ctx, err, types.ErrGettingMaxTallyGasLimit, req) } var gasLimit uint64 if min(req.TallyGasLimit, maxGasLimit) > filterResult.GasUsed { @@ -255,17 +268,22 @@ func (k Keeper) FilterAndTally(ctx sdk.Context, req types.Request) (TallyResult, gasLimit = 0 } + args, err := tallyVMArg(tallyInputs, reveals, filterResult.Outliers) + if err != nil { + return tallyvm.VmResult{}, k.logErrAndRet(ctx, err, types.ErrConstructingTallyVMArgs, req) + } + k.Logger(ctx).Info( "executing tally VM", "request_id", req.ID, "tally_program_id", req.TallyProgramID, - "consensus", result.consensus, + "consensus", filterResult.Consensus, "arguments", args, ) - vmRes := tallyvm.ExecuteTallyVm(tallyProgram.Bytecode, args, map[string]string{ + return tallyvm.ExecuteTallyVm(tallyProgram.Bytecode, args, map[string]string{ "VM_MODE": "tally", - "CONSENSUS": fmt.Sprintf("%v", result.consensus), + "CONSENSUS": fmt.Sprintf("%v", filterResult.Consensus), "BLOCK_HEIGHT": fmt.Sprintf("%d", ctx.BlockHeight()), "DR_ID": req.ID, "DR_REPLICATION_FACTOR": fmt.Sprintf("%v", req.ReplicationFactor), @@ -278,14 +296,7 @@ func (k Keeper) FilterAndTally(ctx sdk.Context, req types.Request) (TallyResult, "DR_GAS_PRICE": req.GasPrice, "DR_MEMO": req.Memo, "DR_PAYBACK_ADDRESS": paybackAddrHex, - }) - result.stdout = vmRes.Stdout - result.stderr = vmRes.Stderr - result.result = vmRes.Result - result.exitInfo = vmRes.ExitInfo - result.tallyGasUsed = vmRes.GasUsed + filterResult.GasUsed - - return result, nil + }), nil } // logErrAndRet logs the base error along with the request ID for diff --git a/x/tally/keeper/filter.go b/x/tally/keeper/filter.go index faee9165..823ba767 100644 --- a/x/tally/keeper/filter.go +++ b/x/tally/keeper/filter.go @@ -1,6 +1,7 @@ package keeper import ( + "encoding/base64" "errors" "fmt" @@ -22,19 +23,48 @@ type FilterResult struct { GasUsed uint64 // gas used for filter } +// BuildFilter builds a filter based on the requestor-provided input. +func (k Keeper) BuildFilter(ctx sdk.Context, filterInput string, replicationFactor uint16) (types.Filter, error) { + input, err := base64.StdEncoding.DecodeString(filterInput) + if err != nil { + return nil, err + } + if len(input) == 0 { + return nil, types.ErrInvalidFilterType + } + + params, err := k.GetParams(ctx) + if err != nil { + return nil, err + } + + var filter types.Filter + switch input[0] { + case filterTypeNone: + filter = types.NewFilterNone(params.FilterGasCostNone) + case filterTypeMode: + filter, err = types.NewFilterMode(input, params.FilterGasCostMultiplierMode, replicationFactor) + case filterTypeStdDev: + filter, err = types.NewFilterStdDev(input, params.FilterGasCostMultiplierStddev, replicationFactor) + default: + return nil, types.ErrInvalidFilterType + } + if err != nil { + return nil, err + } + return filter, nil +} + // ApplyFilter processes filter of the type specified in the first // byte of consensus filter. It returns an outlier list, which is // a boolean list where true at index i means that the reveal at // index i is an outlier, consensus boolean, consensus data proxy // public keys, and error. It assumes that the reveals and their // proxy public keys are sorted. -func (k Keeper) ApplyFilter(ctx sdk.Context, input []byte, reveals []types.RevealBody, replicationFactor uint16) (FilterResult, error) { +func ApplyFilter(filter types.Filter, reveals []types.RevealBody) (FilterResult, error) { var result FilterResult result.Outliers = make([]int, len(reveals)) - - if len(input) == 0 { - return result, types.ErrInvalidFilterType - } + result.GasUsed = filter.GasCost() // Determine basic consensus on tuple of (exit_code, proxy_pub_keys) var maxFreq int @@ -55,30 +85,6 @@ func (k Keeper) ApplyFilter(ctx sdk.Context, input []byte, reveals []types.Revea } result.ProxyPubKeys = proxyPubKeys - params, err := k.GetParams(ctx) - if err != nil { - return result, err - } - - var filter types.Filter - switch input[0] { - case filterTypeNone: - filter, err = types.NewFilterNone(input) - result.GasUsed = params.FilterGasCostNone - case filterTypeMode: - filter, err = types.NewFilterMode(input) - result.GasUsed = params.FilterGasCostMultiplierMode * uint64(replicationFactor) - case filterTypeStdDev: - filter, err = types.NewFilterStdDev(input) - result.GasUsed = params.FilterGasCostMultiplierStddev * uint64(replicationFactor) - default: - return result, types.ErrInvalidFilterType - } - if err != nil { - result.GasUsed = 0 - return result, err - } - outliers, err := filter.ApplyFilter(reveals) switch { case err == nil: diff --git a/x/tally/keeper/filter_fuzz_test.go b/x/tally/keeper/filter_fuzz_test.go index 7bdcc193..429dec2a 100644 --- a/x/tally/keeper/filter_fuzz_test.go +++ b/x/tally/keeper/filter_fuzz_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/require" + "github.com/sedaprotocol/seda-chain/x/tally/keeper" "github.com/sedaprotocol/seda-chain/x/tally/types" ) @@ -58,10 +59,12 @@ func FuzzStdDevFilter(f *testing.F) { bz := make([]byte, 8) binary.BigEndian.PutUint64(bz, uint64(neighborDist*1e6)) filterHex := fmt.Sprintf("02%s01000000000000000b726573756C742E74657874", hex.EncodeToString(bz)) // max_sigma = neighborDist, number_type = int64, json_path = result.text - filter, err := hex.DecodeString(filterHex) + filterInput, err := hex.DecodeString(filterHex) require.NoError(t, err) - result, err := fixture.tallyKeeper.ApplyFilter(fixture.Context(), filter, reveals, uint16(len(reveals))) + filter, err := fixture.tallyKeeper.BuildFilter(fixture.Context(), base64.StdEncoding.EncodeToString(filterInput), uint16(len(reveals))) + require.NoError(t, err) + result, err := keeper.ApplyFilter(filter, reveals) require.Equal(t, expOutliers, result.Outliers) require.ErrorIs(t, err, nil) }) diff --git a/x/tally/keeper/filter_test.go b/x/tally/keeper/filter_test.go index 79925ccb..eca113dd 100644 --- a/x/tally/keeper/filter_test.go +++ b/x/tally/keeper/filter_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" + "github.com/sedaprotocol/seda-chain/x/tally/keeper" "github.com/sedaprotocol/seda-chain/x/tally/types" ) @@ -122,7 +123,7 @@ func TestFilter(t *testing.T) { }, consensus: false, consPubKeys: nil, - gasUsed: 0, + gasUsed: defaultParams.FilterGasCostMultiplierMode * 6, wantErr: types.ErrNoBasicConsensus, }, { @@ -355,7 +356,7 @@ func TestFilter(t *testing.T) { }, consensus: false, consPubKeys: nil, - gasUsed: 0, + gasUsed: defaultParams.FilterGasCostMultiplierMode * 6, wantErr: types.ErrNoBasicConsensus, }, { @@ -430,7 +431,7 @@ func TestFilter(t *testing.T) { }, consensus: false, consPubKeys: nil, - gasUsed: 0, + gasUsed: defaultParams.FilterGasCostMultiplierMode * 4, wantErr: types.ErrNoBasicConsensus, }, { @@ -630,7 +631,7 @@ func TestFilter(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - filter, err := hex.DecodeString(tt.tallyInputAsHex) + filterInput, err := hex.DecodeString(tt.tallyInputAsHex) require.NoError(t, err) // For illustration @@ -643,7 +644,10 @@ func TestFilter(t *testing.T) { sort.Strings(tt.reveals[i].ProxyPubKeys) } - result, err := f.tallyKeeper.ApplyFilter(f.Context(), filter, tt.reveals, uint16(len(tt.reveals))) + filter, err := f.tallyKeeper.BuildFilter(f.Context(), base64.StdEncoding.EncodeToString(filterInput), uint16(len(tt.reveals))) + require.NoError(t, err) + + result, err := keeper.ApplyFilter(filter, tt.reveals) require.ErrorIs(t, err, tt.wantErr) if tt.consPubKeys == nil { require.Nil(t, nil, result.ProxyPubKeys) diff --git a/x/tally/types/codec_test.go b/x/tally/types/codec_test.go index ed48d48f..cb4ae772 100644 --- a/x/tally/types/codec_test.go +++ b/x/tally/types/codec_test.go @@ -50,7 +50,7 @@ func TestDecodeFilterInput(t *testing.T) { b, err := hex.DecodeString(tt.hexStr) require.NoError(t, err) - filter, err := NewFilterMode(b) + filter, err := NewFilterMode(b, 1, 1) if tt.wantErr != nil { require.ErrorIs(t, err, tt.wantErr) return diff --git a/x/tally/types/filters.go b/x/tally/types/filters.go index d68d04c3..4a38ea41 100644 --- a/x/tally/types/filters.go +++ b/x/tally/types/filters.go @@ -8,26 +8,36 @@ import ( "golang.org/x/exp/constraints" ) +var ( + _ Filter = &FilterNone{} + _ Filter = &FilterMode{} + _ Filter = &FilterStdDev{} +) + type Filter interface { // ApplyFilter takes in a list of reveals and returns an outlier // list, whose value at index i indicates whether i-th reveal is // an outlier. Value of 1 indicates an outlier, and value of 0 // indicates a non-outlier reveal. ApplyFilter(reveals []RevealBody) ([]int, error) + // GasCost returns the cost of the filter in terms of gas amount. + GasCost() uint64 } -type FilterNone struct{} +type FilterNone struct { + gasCost uint64 +} // NewFilterNone constructs a new FilterNone object. -func NewFilterNone(_ []byte) (FilterNone, error) { - return FilterNone{}, nil +func NewFilterNone(gasCost uint64) FilterNone { + return FilterNone{gasCost: gasCost} } // FilterNone declares all reveals as non-outliers, unless reveals are // empty or corrupt. func (f FilterNone) ApplyFilter(reveals []RevealBody) ([]int, error) { if len(reveals) == 0 { - return nil, ErrEmptyReveals + return nil, ErrEmptyReveals // TODO remove since unreachable? } var corruptCount int @@ -44,8 +54,13 @@ func (f FilterNone) ApplyFilter(reveals []RevealBody) ([]int, error) { return make([]int, len(reveals)), nil } +func (f FilterNone) GasCost() uint64 { + return f.gasCost +} + type FilterMode struct { dataPath string // JSON path to reveal data + gasCost uint64 } // NewFilterMode constructs a new FilerMode object given a filter @@ -53,7 +68,7 @@ type FilterMode struct { // Mode filter input looks as follows: // 0 1 9 9+data_path_length // | filter_type | data_path_length | data_path | -func NewFilterMode(input []byte) (FilterMode, error) { +func NewFilterMode(input []byte, gasCostMultiplier uint64, replicationFactor uint16) (FilterMode, error) { var filter FilterMode if len(input) < 9 { return filter, ErrFilterInputTooShort.Wrapf("%d < %d", len(input), 9) @@ -70,9 +85,14 @@ func NewFilterMode(input []byte) (FilterMode, error) { return filter, ErrInvalidPathLen.Wrapf("expected: %d got: %d", int(pathLen), len(path)) // #nosec G115 } filter.dataPath = string(path) + filter.gasCost = gasCostMultiplier * uint64(replicationFactor) return filter, nil } +func (f FilterMode) GasCost() uint64 { + return f.gasCost +} + // ApplyFilter applies the Mode Filter and returns an outlier list. // (i) If more than 1/3 of reveals are corrupted, a corrupt reveals // error is returned without an outlier list. @@ -99,8 +119,9 @@ func (f FilterMode) ApplyFilter(reveals []RevealBody) ([]int, error) { type FilterStdDev struct { maxSigma Sigma - numberType byte dataPath string // JSON path to reveal data + filterFunc func(dataList []any, maxSigma Sigma) ([]int, error) + gasCost uint64 } // NewFilterStdDev constructs a new FilterStdDev object given a @@ -108,7 +129,7 @@ type FilterStdDev struct { // Standard deviation filter input looks as follows: // 0 1 9 10 18 18+json_path_length // | filter_type | max_sigma | number_type | json_path_length | json_path | -func NewFilterStdDev(input []byte) (FilterStdDev, error) { +func NewFilterStdDev(input []byte, gasCostMultiplier uint64, replicationFactor uint16) (FilterStdDev, error) { var filter FilterStdDev if len(input) < 18 { return filter, ErrFilterInputTooShort.Wrapf("%d < %d", len(input), 18) @@ -120,7 +141,18 @@ func NewFilterStdDev(input []byte) (FilterStdDev, error) { } filter.maxSigma = maxSigma - filter.numberType = input[9] + switch input[9] { + case 0x00: // Int32 + filter.filterFunc = detectOutliersInteger[int32] + case 0x01: // Int64 + filter.filterFunc = detectOutliersInteger[int64] + case 0x02: // Uint32 + filter.filterFunc = detectOutliersInteger[uint32] + case 0x03: // Uint64 + filter.filterFunc = detectOutliersInteger[uint64] + default: + return filter, ErrInvalidNumberType + } var pathLen uint64 err = binary.Read(bytes.NewReader(input[10:18]), binary.BigEndian, &pathLen) @@ -133,6 +165,7 @@ func NewFilterStdDev(input []byte) (FilterStdDev, error) { return filter, ErrInvalidPathLen.Wrapf("expected: %d got: %d", int(pathLen), len(path)) // #nosec G115 } filter.dataPath = string(path) + filter.gasCost = gasCostMultiplier * uint64(replicationFactor) return filter, nil } @@ -152,19 +185,11 @@ func (f FilterStdDev) ApplyFilter(reveals []RevealBody) ([]int, error) { if err != nil { return nil, err } + return f.filterFunc(dataList, f.maxSigma) +} - switch f.numberType { - case 0x00: // Int32 - return detectOutliersInteger[int32](dataList, f.maxSigma) - case 0x01: // Int64 - return detectOutliersInteger[int64](dataList, f.maxSigma) - case 0x02: // Uint32 - return detectOutliersInteger[uint32](dataList, f.maxSigma) - case 0x03: // Uint64 - return detectOutliersInteger[uint64](dataList, f.maxSigma) - default: - return nil, ErrInvalidNumberType - } +func (f FilterStdDev) GasCost() uint64 { + return f.gasCost } func detectOutliersInteger[T constraints.Integer](dataList []any, maxSigma Sigma) ([]int, error) { From dd03bc439cc2c31139b1507d653c1ccdf26a1142 Mon Sep 17 00:00:00 2001 From: hacheigriega Date: Mon, 6 Jan 2025 15:36:19 +0900 Subject: [PATCH 2/3] refactor(x/tally): use errors list and simplify filtering errors --- x/tally/keeper/endblock.go | 4 +- x/tally/keeper/filter.go | 50 +++++++++++++--------- x/tally/keeper/filter_test.go | 66 +++++++++++++---------------- x/tally/types/errors.go | 27 ++++++------ x/tally/types/filters.go | 78 +++++++++++------------------------ x/tally/types/filters_util.go | 35 ++++++---------- 6 files changed, 110 insertions(+), 150 deletions(-) diff --git a/x/tally/keeper/endblock.go b/x/tally/keeper/endblock.go index 1fe1f097..bfd035ec 100644 --- a/x/tally/keeper/endblock.go +++ b/x/tally/keeper/endblock.go @@ -94,11 +94,11 @@ func (k Keeper) ProcessTallies(ctx sdk.Context, coreContract sdk.AccAddress) err } switch { - case len(req.Commits) < int(req.ReplicationFactor): + 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 = batchingtypes.TallyExitCodeNotEnoughCommits k.Logger(ctx).Info("data request's number of commits did not meet replication factor", "request_id", req.ID) - case len(req.Reveals) < int(req.ReplicationFactor): + 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 = batchingtypes.TallyExitCodeNotEnoughReveals k.Logger(ctx).Info("data request's number of reveals did not meet replication factor", "request_id", req.ID) diff --git a/x/tally/keeper/filter.go b/x/tally/keeper/filter.go index 823ba767..9cda533d 100644 --- a/x/tally/keeper/filter.go +++ b/x/tally/keeper/filter.go @@ -2,7 +2,6 @@ package keeper import ( "encoding/base64" - "errors" "fmt" sdk "github.com/cosmos/cosmos-sdk/types" @@ -17,10 +16,21 @@ const ( ) type FilterResult struct { - Outliers []int // outlier list - Consensus bool // whether consensus was reached - ProxyPubKeys []string // consensus data proxy public keys - GasUsed uint64 // gas used for filter + Errors []bool // i-th item is true if i-th reveal is non-zero exit or corrupt + Outliers []int // i-th item is non-zero if i-th reveal is an outlier + Consensus bool // whether consensus (either in data or in error) is reached + ProxyPubKeys []string // data proxy public keys in consensus + GasUsed uint64 // gas used by filter +} + +func errorCount(errors []bool) int { + count := 0 + for _, err := range errors { + if err { + count++ + } + } + return count } // BuildFilter builds a filter based on the requestor-provided input. @@ -63,40 +73,40 @@ func (k Keeper) BuildFilter(ctx sdk.Context, filterInput string, replicationFact // proxy public keys are sorted. func ApplyFilter(filter types.Filter, reveals []types.RevealBody) (FilterResult, error) { var result FilterResult + result.Errors = make([]bool, len(reveals)) result.Outliers = make([]int, len(reveals)) result.GasUsed = filter.GasCost() - // Determine basic consensus on tuple of (exit_code, proxy_pub_keys) + // Determine basic consensus on tuple of (exit_code_success, proxy_pub_keys) var maxFreq int - var proxyPubKeys []string freq := make(map[string]int, len(reveals)) - for _, reveal := range reveals { + for i, reveal := range reveals { success := reveal.ExitCode == 0 + result.Errors[i] = !success tuple := fmt.Sprintf("%v%v", success, reveal.ProxyPubKeys) freq[tuple]++ if freq[tuple] > maxFreq { - proxyPubKeys = reveal.ProxyPubKeys + result.ProxyPubKeys = reveal.ProxyPubKeys maxFreq = freq[tuple] } } - if maxFreq*3 < len(reveals)*2 { + if maxFreq*3 < len(reveals)*2 { // TODO remove basic consensus check? return result, types.ErrNoBasicConsensus } - result.ProxyPubKeys = proxyPubKeys - outliers, err := filter.ApplyFilter(reveals) + outliers, consensus := filter.ApplyFilter(reveals, result.Errors) + switch { - case err == nil: - result.Outliers = outliers + case errorCount(result.Errors)*3 > len(reveals)*2: + result.Consensus = true + return result, types.ErrConsensusInError + case !consensus: + result.Consensus = false + return result, types.ErrNoConsensus + default: result.Consensus = true - return result, nil - case errors.Is(err, types.ErrNoConsensus): result.Outliers = outliers return result, nil - case errors.Is(err, types.ErrCorruptReveals): - return result, err - default: - return result, err } } diff --git a/x/tally/keeper/filter_test.go b/x/tally/keeper/filter_test.go index eca113dd..5f1c4dfe 100644 --- a/x/tally/keeper/filter_test.go +++ b/x/tally/keeper/filter_test.go @@ -32,7 +32,7 @@ func TestFilter(t *testing.T) { { name: "None filter", tallyInputAsHex: "00", - outliers: []int{0, 0, 0, 0, 0}, + outliers: make([]int, 5), reveals: []types.RevealBody{ {}, {}, @@ -80,7 +80,7 @@ func TestFilter(t *testing.T) { { name: "Mode filter - Multiple modes", tallyInputAsHex: "01000000000000000D242E726573756C742E74657874", // json_path = $.result.text - outliers: []int{0, 0, 0, 0, 0, 0, 1}, + outliers: make([]int, 7), reveals: []types.RevealBody{ {Reveal: `{"result": {"text": "A"}}`}, {Reveal: `{"result": {"text": "A"}}`}, @@ -93,7 +93,7 @@ func TestFilter(t *testing.T) { consensus: false, consPubKeys: nil, gasUsed: defaultParams.FilterGasCostMultiplierMode * 7, - wantErr: nil, + wantErr: types.ErrNoConsensus, }, { name: "Mode filter - One corrupt reveal but consensus", @@ -112,7 +112,7 @@ func TestFilter(t *testing.T) { { name: "Mode filter - No consensus on exit code", tallyInputAsHex: "01000000000000000D242E726573756C742E74657874", // json_path = $.result.text - outliers: []int{0, 0, 0, 0, 0, 0}, + outliers: make([]int, 6), reveals: []types.RevealBody{ {ExitCode: 1, Reveal: `{"high_level_prop1":"ignore this", "result": {"text": "A", "number": 0}}`}, {ExitCode: 1, Reveal: `{"makes_this_json":"ignore this", "result": {"text": "A", "number": 10}}`}, @@ -127,26 +127,26 @@ func TestFilter(t *testing.T) { wantErr: types.ErrNoBasicConsensus, }, { - name: "Mode filter - Corrupt due to too many bad exit codes", + name: "Mode filter - >2/3 bad exit codes", tallyInputAsHex: "01000000000000000D242E726573756C742E74657874", // json_path = $.result.text - outliers: []int{0, 0, 0, 0, 0, 0}, + outliers: make([]int, 6), reveals: []types.RevealBody{ {ExitCode: 1, Reveal: `{"high_level_prop1":"ignore this", "result": {"text": "A", "number": 0}}`}, {ExitCode: 1, Reveal: `{"makes_this_json":"ignore this", "result": {"text": "A", "number": 10}}`}, {ExitCode: 1, Reveal: `{"unstructured":"ignore this", "result": {"text": "B", "number": 101}}`}, {ExitCode: 1, Reveal: `{"but":"ignore this", "result": {"text": "B", "number": 10}}`}, {ExitCode: 0, Reveal: `{"it_does_not":"ignore this", "result": {"text": "C", "number": 10}}`}, - {ExitCode: 0, Reveal: `{"matter":"ignore this", "result": {"text": "C", "number": 10}}`}, + {ExitCode: 1, Reveal: `{"matter":"ignore this", "result": {"text": "C", "number": 10}}`}, }, - consensus: false, + consensus: true, consPubKeys: nil, gasUsed: defaultParams.FilterGasCostMultiplierMode * 6, - wantErr: types.ErrCorruptReveals, + wantErr: types.ErrConsensusInError, }, { name: "Mode filter - Uniform reveals", tallyInputAsHex: "01000000000000000D242E726573756C742E74657874", // json_path = $.result.text - outliers: []int{0, 0, 0, 0, 0, 0}, + outliers: make([]int, 6), reveals: []types.RevealBody{ { ExitCode: 0, @@ -220,7 +220,7 @@ func TestFilter(t *testing.T) { wantErr: nil, }, { - name: "Mode filter - Basic consensus but corrupt due to too many bad exit codes", + name: "Mode filter - >2/3 bad exit codes", tallyInputAsHex: "01000000000000000D242E726573756C742E74657874", // json_path = $.result.text outliers: []int{0, 0, 0, 0, 0, 0}, reveals: []types.RevealBody{ @@ -284,7 +284,7 @@ func TestFilter(t *testing.T) { Reveal: `{"result": {"text": "A"}}`, }, }, - consensus: false, + consensus: true, consPubKeys: []string{ "02100efce2a783cc7a3fbf9c5d15d4cc6e263337651312f21a35d30c16cb38f4g3", "034c0f86f0cb61f9ddb47c4ba0b2ca0470962b5a1c50bee3a563184979672195f4", @@ -292,12 +292,12 @@ func TestFilter(t *testing.T) { "034c0f86f0cb61f9ddb47c4ba0b2ca0470962b5a1c50bee3a563184979672195f4", }, gasUsed: defaultParams.FilterGasCostMultiplierMode * 6, - wantErr: types.ErrCorruptReveals, + wantErr: types.ErrConsensusInError, }, { name: "Mode filter with proxy pubkeys - No basic consensus", tallyInputAsHex: "01000000000000000D242E726573756C742E74657874", // json_path = $.result.text - outliers: []int{0, 0, 0, 0, 0, 0}, + outliers: make([]int, 6), reveals: []types.RevealBody{ { ExitCode: 1, @@ -377,7 +377,7 @@ func TestFilter(t *testing.T) { { name: "Mode filter - No consensus due to non-zero exit code invalidating data", tallyInputAsHex: "01000000000000000D242E726573756C742E74657874", // json_path = $.result.text - outliers: []int{0, 0, 1, 1}, + outliers: make([]int, 4), reveals: []types.RevealBody{ {ExitCode: 0, ProxyPubKeys: []string{"02100efce2a783cc7a3fbf9c5d15d4cc6e263337651312f21a35d30c16cb38f4g3"}, Reveal: `{"result": {"text": "mac"}}`}, {ExitCode: 0, ProxyPubKeys: []string{"02100efce2a783cc7a3fbf9c5d15d4cc6e263337651312f21a35d30c16cb38f4g3"}, Reveal: `{"result": {"text": "mac"}}`}, @@ -387,12 +387,12 @@ func TestFilter(t *testing.T) { consensus: false, consPubKeys: []string{"02100efce2a783cc7a3fbf9c5d15d4cc6e263337651312f21a35d30c16cb38f4g3"}, gasUsed: defaultParams.FilterGasCostMultiplierMode * 4, - wantErr: nil, + wantErr: types.ErrNoConsensus, }, { name: "Mode filter - No consensus with exit code invalidating a reveal", tallyInputAsHex: "01000000000000000D242E726573756C742E74657874", // json_path = $.result.text - outliers: []int{0, 0, 0, 1}, + outliers: make([]int, 4), reveals: []types.RevealBody{ {ExitCode: 0, ProxyPubKeys: []string{"02100efce2a783cc7a3fbf9c5d15d4cc6e263337651312f21a35d30c16cb38f4g3"}, Reveal: `{"result": {"text": "mac"}}`}, {ExitCode: 0, ProxyPubKeys: []string{"02100efce2a783cc7a3fbf9c5d15d4cc6e263337651312f21a35d30c16cb38f4g3"}, Reveal: `{"result": {"text": ""}}`}, @@ -402,7 +402,7 @@ func TestFilter(t *testing.T) { consensus: false, consPubKeys: []string{"02100efce2a783cc7a3fbf9c5d15d4cc6e263337651312f21a35d30c16cb38f4g3"}, gasUsed: defaultParams.FilterGasCostMultiplierMode * 4, - wantErr: nil, + wantErr: types.ErrNoConsensus, }, { name: "Mode filter - One reports bad pubkey but is not an outlier", @@ -458,7 +458,7 @@ func TestFilter(t *testing.T) { { name: "Mode filter - Consensus not reached due to exit code", tallyInputAsHex: "01000000000000000D242E726573756C742E74657874", // json_path = $.result.text - outliers: []int{1, 0, 0, 1, 1, 0}, + outliers: make([]int, 6), reveals: []types.RevealBody{ {Reveal: `{"result": {"text": "A", "number": 0}}`, ExitCode: 1}, {Reveal: `{"result": {"text": "A", "number": 0}}`}, @@ -470,12 +470,12 @@ func TestFilter(t *testing.T) { consensus: false, consPubKeys: nil, gasUsed: defaultParams.FilterGasCostMultiplierMode * 6, - wantErr: nil, + wantErr: types.ErrNoConsensus, }, { name: "Mode filter - Consensus not reached due to corrupt reveal", tallyInputAsHex: "01000000000000000D242E726573756C742E74657874", // json_path = $.result.text - outliers: []int{1, 0, 0, 1, 1, 0}, + outliers: make([]int, 6), reveals: []types.RevealBody{ {Reveal: `{"resalt": {"text": "A", "number": 0}}`}, {Reveal: `{"result": {"text": "A", "number": 10}}`}, @@ -487,7 +487,7 @@ func TestFilter(t *testing.T) { consensus: false, consPubKeys: nil, gasUsed: defaultParams.FilterGasCostMultiplierMode * 6, - wantErr: nil, + wantErr: types.ErrNoConsensus, }, { name: "Standard deviation filter uint64", @@ -523,16 +523,6 @@ func TestFilter(t *testing.T) { gasUsed: defaultParams.FilterGasCostMultiplierStddev * 6, wantErr: nil, }, - { - name: "Standard deviation filter - Empty reveal", - tallyInputAsHex: "02000000000016E36001000000000000000D242E726573756C742E74657874", // max_sigma = 1.5, number_type = uint64, json_path = $.result.text - outliers: []int{}, - reveals: []types.RevealBody{}, - consensus: false, - consPubKeys: nil, - gasUsed: 0, - wantErr: types.ErrEmptyReveals, - }, { name: "Standard deviation filter - Single reveal", tallyInputAsHex: "02000000000016E36001000000000000000D242E726573756C742E74657874", // max_sigma = 1.5, number_type = uint64, json_path = $.result.text @@ -548,7 +538,7 @@ func TestFilter(t *testing.T) { { name: "Standard deviation filter - One corrupt reveal", tallyInputAsHex: "02000000000016E36001000000000000000D242E726573756C742E74657874", // max_sigma = 1.5, number_type = uint64, json_path = $.result.text - outliers: []int{1, 0, 0, 0, 1, 1}, + outliers: make([]int, 6), reveals: []types.RevealBody{ {Reveal: `{"result": {"text": 4, "number": 0}}`}, {Reveal: `{"result": {"text": 5, "number": 10}}`}, @@ -560,7 +550,7 @@ func TestFilter(t *testing.T) { consensus: false, consPubKeys: nil, gasUsed: defaultParams.FilterGasCostMultiplierStddev * 6, - wantErr: nil, + wantErr: types.ErrNoConsensus, }, { name: "Standard deviation filter - Max sigma 1.55", @@ -582,7 +572,7 @@ func TestFilter(t *testing.T) { { name: "Standard deviation filter - Max sigma 1.45", tallyInputAsHex: "02000000000016201003000000000000000D242E726573756C742E74657874", // max_sigma = 1.45, number_type = uint64, json_path = $.result.text - outliers: []int{1, 1, 0, 0, 1, 1}, + outliers: make([]int, 6), reveals: []types.RevealBody{ {Reveal: `{"result": {"text": 4, "number": 0}}`}, {Reveal: `{"result": {"text": 5, "number": 10}}`}, @@ -594,7 +584,7 @@ func TestFilter(t *testing.T) { consensus: false, consPubKeys: nil, gasUsed: defaultParams.FilterGasCostMultiplierStddev * 6, - wantErr: nil, + wantErr: types.ErrNoConsensus, }, { name: "Standard deviation filter int64 with negative reveals", @@ -616,7 +606,7 @@ func TestFilter(t *testing.T) { { name: "Standard deviation filter int64 median -0.5", tallyInputAsHex: "02000000000007A12001000000000000000D242E726573756C742E74657874", // max_sigma = 0.5, number_type = int64, json_path = $.result.text - outliers: []int{1, 0, 0, 1}, + outliers: make([]int, 4), reveals: []types.RevealBody{ {Reveal: `{"result": {"text": 1, "number": 0}}`}, {Reveal: `{"result": {"text": 0, "number": 0}}`}, @@ -626,7 +616,7 @@ func TestFilter(t *testing.T) { consensus: false, consPubKeys: nil, gasUsed: defaultParams.FilterGasCostMultiplierStddev * 4, - wantErr: nil, + wantErr: types.ErrNoConsensus, }, } for _, tt := range tests { diff --git a/x/tally/types/errors.go b/x/tally/types/errors.go index c6894ae3..5ecd98e3 100644 --- a/x/tally/types/errors.go +++ b/x/tally/types/errors.go @@ -3,22 +3,19 @@ package types import "cosmossdk.io/errors" var ( + // Errors used in filter: ErrInvalidFilterType = errors.Register("tally", 2, "invalid filter type") ErrFilterInputTooShort = errors.Register("tally", 3, "filter input length too short") ErrInvalidPathLen = errors.Register("tally", 4, "invalid JSON path length") - ErrEmptyReveals = errors.Register("tally", 5, "no reveals given") - ErrCorruptReveals = errors.Register("tally", 6, "> 1/3 of reveals are corrupted") - ErrNoConsensus = errors.Register("tally", 7, "> 1/3 of reveals do not agree on reveal data") - ErrNoBasicConsensus = errors.Register("tally", 8, "> 1/3 of reveals do not agree on (exit_code, proxy_pub_keys)") - ErrInvalidNumberType = errors.Register("tally", 9, "invalid number type specified") - ErrFilterUnexpected = errors.Register("tally", 10, "unexpected error occurred in filter") - ErrInvalidSaltLength = errors.Register("tally", 11, "salt should be 32-byte long") - // Errors from FilterAndTally: - ErrDecodingConsensusFilter = errors.Register("tally", 12, "failed to decode consensus filter") - ErrDecodingPaybackAddress = errors.Register("tally", 13, "failed to decode payback address") - ErrApplyingFilter = errors.Register("tally", 14, "failed to apply filter") - ErrFindingTallyProgram = errors.Register("tally", 15, "failed to find tally program") - ErrDecodingTallyInputs = errors.Register("tally", 16, "failed to decode tally inputs") - ErrConstructingTallyVMArgs = errors.Register("tally", 17, "failed to construct tally VM arguments") - ErrGettingMaxTallyGasLimit = errors.Register("tally", 18, "failed to get max tally gas limit") + ErrInvalidNumberType = errors.Register("tally", 5, "invalid number type specified") + ErrApplyingFilter = errors.Register("tally", 6, "failed to apply filter") + ErrConsensusInError = errors.Register("tally", 7, "consensus in error") + ErrNoConsensus = errors.Register("tally", 8, "> 1/3 of reveals do not agree on reveal data") + ErrNoBasicConsensus = errors.Register("tally", 9, "> 1/3 of reveals do not agree on (exit_code_success, proxy_pub_keys)") + // Errors used in tally execution: + ErrDecodingPaybackAddress = errors.Register("tally", 10, "failed to decode payback address") + ErrFindingTallyProgram = errors.Register("tally", 11, "failed to find tally program") + ErrDecodingTallyInputs = errors.Register("tally", 12, "failed to decode tally inputs") + ErrConstructingTallyVMArgs = errors.Register("tally", 13, "failed to construct tally VM arguments") + ErrGettingMaxTallyGasLimit = errors.Register("tally", 14, "failed to get max tally gas limit") ) diff --git a/x/tally/types/filters.go b/x/tally/types/filters.go index 4a38ea41..5b86b2ec 100644 --- a/x/tally/types/filters.go +++ b/x/tally/types/filters.go @@ -17,9 +17,9 @@ var ( type Filter interface { // ApplyFilter takes in a list of reveals and returns an outlier // list, whose value at index i indicates whether i-th reveal is - // an outlier. Value of 1 indicates an outlier, and value of 0 - // indicates a non-outlier reveal. - ApplyFilter(reveals []RevealBody) ([]int, error) + // an outlier, and a boolean indicating whether consensus in reveal + // data has been reached. + ApplyFilter(reveals []RevealBody, errors []bool) ([]int, bool) // GasCost returns the cost of the filter in terms of gas amount. GasCost() uint64 } @@ -33,25 +33,9 @@ func NewFilterNone(gasCost uint64) FilterNone { return FilterNone{gasCost: gasCost} } -// FilterNone declares all reveals as non-outliers, unless reveals are -// empty or corrupt. -func (f FilterNone) ApplyFilter(reveals []RevealBody) ([]int, error) { - if len(reveals) == 0 { - return nil, ErrEmptyReveals // TODO remove since unreachable? - } - - var corruptCount int - for _, r := range reveals { - if r.ExitCode != 0 { - corruptCount++ - continue - } - } - if corruptCount*3 > len(reveals) { - return nil, ErrCorruptReveals - } - - return make([]int, len(reveals)), nil +// FilterNone declares all reveals as non-outliers. +func (f FilterNone) ApplyFilter(reveals []RevealBody, _ []bool) ([]int, bool) { + return make([]int, len(reveals)), true } func (f FilterNone) GasCost() uint64 { @@ -94,16 +78,11 @@ func (f FilterMode) GasCost() uint64 { } // ApplyFilter applies the Mode Filter and returns an outlier list. -// (i) If more than 1/3 of reveals are corrupted, a corrupt reveals -// error is returned without an outlier list. -// (ii) Otherwise, a reveal is declared an outlier if it does not -// match the mode value. If less than 2/3 of the reveals are non-outliers, -// "no consensus" error is returned along with an outlier list. -func (f FilterMode) ApplyFilter(reveals []RevealBody) ([]int, error) { - dataList, dataAttrs, err := parseReveals(reveals, f.dataPath) - if err != nil { - return nil, err - } +// A reveal is declared an outlier if it does not match the mode value. +// If less than 2/3 of the reveals are non-outliers, "no consensus" +// error is returned along with an outlier list. +func (f FilterMode) ApplyFilter(reveals []RevealBody, errors []bool) ([]int, bool) { + dataList, dataAttrs := parseReveals(reveals, f.dataPath, errors) outliers := make([]int, len(reveals)) for i, r := range dataList { @@ -112,15 +91,15 @@ func (f FilterMode) ApplyFilter(reveals []RevealBody) ([]int, error) { } } if dataAttrs.maxFreq*3 < len(reveals)*2 { - return outliers, ErrNoConsensus + return outliers, false } - return outliers, nil + return outliers, true } type FilterStdDev struct { maxSigma Sigma dataPath string // JSON path to reveal data - filterFunc func(dataList []any, maxSigma Sigma) ([]int, error) + filterFunc func(dataList []any, maxSigma Sigma, errors []bool) ([]int, bool) gasCost uint64 } @@ -180,38 +159,31 @@ func NewFilterStdDev(input []byte, gasCostMultiplier uint64, replicationFactor u // an outlier if it deviates from the median by more than the given // max sigma. If less than 2/3 of the reveals are non-outliers, "no // consensus" error is returned as well. -func (f FilterStdDev) ApplyFilter(reveals []RevealBody) ([]int, error) { - dataList, _, err := parseReveals(reveals, f.dataPath) - if err != nil { - return nil, err - } - return f.filterFunc(dataList, f.maxSigma) +func (f FilterStdDev) ApplyFilter(reveals []RevealBody, errors []bool) ([]int, bool) { + dataList, _ := parseReveals(reveals, f.dataPath, errors) + return f.filterFunc(dataList, f.maxSigma, errors) } func (f FilterStdDev) GasCost() uint64 { return f.gasCost } -func detectOutliersInteger[T constraints.Integer](dataList []any, maxSigma Sigma) ([]int, error) { +func detectOutliersInteger[T constraints.Integer](dataList []any, maxSigma Sigma, errors []bool) ([]int, bool) { nums := make([]T, 0, len(dataList)) corruptQueue := make([]int, 0, len(dataList)) // queue of corrupt indices in dataList for i, data := range dataList { if data == nil { + errors[i] = true corruptQueue = append(corruptQueue, i) continue } - num, ok := data.(int64) + num, ok := data.(T) if !ok { + errors[i] = true corruptQueue = append(corruptQueue, i) continue } - nums = append(nums, T(num)) - } - - // If more than 1/3 of the reveals are corrupted, - // return corrupt reveals error. - if len(corruptQueue)*3 > len(dataList) { - return nil, ErrCorruptReveals + nums = append(nums, num) } // Construct outliers list. @@ -233,11 +205,11 @@ func detectOutliersInteger[T constraints.Integer](dataList []any, maxSigma Sigma } // If less than 2/3 of the numbers fall within max sigma range - // from the median, there is no consensus. + // from the median, there is no consensus in reveal data. if nonOutlierCount*3 < len(nums)*2 { - return outliers, ErrNoConsensus + return outliers, false } - return outliers, nil + return outliers, true } // findMedian returns the median of a given slice of integers. diff --git a/x/tally/types/filters_util.go b/x/tally/types/filters_util.go index 9c9a87d5..e2c9f529 100644 --- a/x/tally/types/filters_util.go +++ b/x/tally/types/filters_util.go @@ -12,45 +12,39 @@ type dataAttributes struct { maxFreq int // frequency of most frequent data in data list } -// parseReveals parses a list of RevealBody objects using a given data -// path and returns a data list. However, if more than 1/3 of the reveals -// are corrupted (i.e. cannot be parsed), no data list is returned and -// ErrCorruptReveals error is returned. When there is no error, it also -// returns dataAttributes struct since some filters require this information. -// Note that when an i-th reveal is corrupted, the i-th item in the data -// list is left as an empty string. -func parseReveals(reveals []RevealBody, dataPath string) ([]any, dataAttributes, error) { - if len(reveals) == 0 { - return nil, dataAttributes{}, ErrEmptyReveals - } - - var maxFreq, corruptCount int +// parseReveals parses a list of RevealBody objects using the given +// data path and returns a parsed data list along with its attributes. +// It also updates the given errors list to indicate true for the items +// that are corrupted. Note when an i-th reveal is corrupted, the i-th +// item in the data list is left as nil. +func parseReveals(reveals []RevealBody, dataPath string, errors []bool) ([]any, dataAttributes) { + var maxFreq int freq := make(map[any]int, len(reveals)) dataList := make([]any, len(reveals)) for i, r := range reveals { if r.ExitCode != 0 { - corruptCount++ + errors[i] = true continue } revealBytes, err := base64.StdEncoding.DecodeString(r.Reveal) if err != nil { - corruptCount++ + errors[i] = true continue } obj, err := oj.Parse(revealBytes) if err != nil { - corruptCount++ + errors[i] = true continue } expr, err := jp.ParseString(dataPath) if err != nil { - corruptCount++ + errors[i] = true continue } elems := expr.Get(obj) if len(elems) < 1 { - corruptCount++ + errors[i] = true continue } data := elems[0] @@ -60,11 +54,8 @@ func parseReveals(reveals []RevealBody, dataPath string) ([]any, dataAttributes, dataList[i] = data } - if corruptCount*3 > len(reveals) { - return nil, dataAttributes{}, ErrCorruptReveals - } return dataList, dataAttributes{ freqMap: freq, maxFreq: maxFreq, - }, nil + } } From 7100b4f47ee2e732f7d10cb193a113cce467e72c Mon Sep 17 00:00:00 2001 From: hacheigriega Date: Mon, 6 Jan 2025 19:06:06 +0900 Subject: [PATCH 3/3] fix(x/tally): use int64 concrete type for std dev filter and add length check of parsed numbers --- x/tally/types/filters.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/x/tally/types/filters.go b/x/tally/types/filters.go index 5b86b2ec..c1f51ec3 100644 --- a/x/tally/types/filters.go +++ b/x/tally/types/filters.go @@ -177,18 +177,21 @@ func detectOutliersInteger[T constraints.Integer](dataList []any, maxSigma Sigma corruptQueue = append(corruptQueue, i) continue } - num, ok := data.(T) + num, ok := data.(int64) if !ok { errors[i] = true corruptQueue = append(corruptQueue, i) continue } - nums = append(nums, num) + nums = append(nums, T(num)) } // Construct outliers list. - median := findMedian(nums) outliers := make([]int, len(dataList)) + if len(nums) == 0 { + return outliers, false + } + median := findMedian(nums) var numsInd, nonOutlierCount int for i := range outliers { if len(corruptQueue) > 0 && i == corruptQueue[0] {