feat: Refactor filtering and adding std dev filter #300

hacheigriega · 2024-07-09T16:42:10Z

Explanation of Changes

Fixing issues identified during devnet testing

Consensus is always set to false if tally VM does not run even if filtering result gives consensus = true.
Handle the case when reveal value is not found at a given JSON path.
Invalid JSON path, invalid reveal encoding, and non-zero exit code should all be treated as "corrupt reveals."
Add events
Hex-encode tally VM inputs.

This PR also refactors filtering and adds a new filter standard deviation.

Make use of a new interface type Filter to clean up the code.
Add standard deviation filter

Closes: #284
Closes: #286

hacheigriega · 2024-07-09T17:24:00Z

x/wasm-storage/keeper/filter.go

 	if maxFreq*3 < len(reveals)*2 {
 		return outliers, false, nil
 	}

 	for i, r := range revealVals {
-		if freq[r] != maxFreq || reveals[i].ExitCode != 0 {
-			continue
+		if freq[r] == maxFreq {


My reasoning was that since we know at this point that

Corrupt reveals are <= 1/3

Max frequency occurs >= 2/3

=> corrupt reveals can never have the frequency of maxFreq.

Thomasvdam

At a high level the code makes sense to me (as names of functions, location in files, etc). I need to take a real close look at the filters: how they determine consensus and if this aligns with what Franklin and Mario have in mind. I'm not sharp enough right now, so I'll have a look tomorrow together with Franklin.

hacheigriega · 2024-07-15T14:56:22Z

@Thomasvdam Sounds good and thanks. I'll have this PR fully ready for review by then. I will also share a flow chart I drew.

hacheigriega · 2024-07-15T18:33:21Z

The specs are outlined here https://www.notion.so/sedaprotocol/Tallying-9388fd590d1c4b659abb721b57909feb?pvs=4#114307cdfc9d4c8b929dc11deb540ca3

x/wasm-storage/keeper/abci.go

x/wasm-storage/keeper/filter.go

x/wasm-storage/types/filters.go

FranklinWaller · 2024-07-16T12:24:44Z

Review was together done with @Thomasvdam btw

hacheigriega · 2024-07-18T18:01:13Z

Ready for final review. The simulation test fails due to its inherent instability. We will have to wait for Cosmos team's simulation test refactoring.

x/wasm-storage/keeper/abci_test.go

x/wasm-storage/keeper/filter_test.go

x/wasm-storage/types/sigma.go

x/wasm-storage/types/filters.go

hacheigriega · 2024-07-19T16:19:05Z

@Thomasvdam Oh yes you're right. I thought we were assuming that the results would be base64-encoded, but I realized that I came up with that assumption myself. We should probably use JSON number for this instead, huh?

Thomasvdam · 2024-07-19T16:31:33Z

Yeah I think so :)

hacheigriega · 2024-07-19T18:49:47Z

Yeah I think so :)

Fixed in 482764d. ~~parseReveals() parses any values into strings, which then are parsed into integers using strconv functions in the standard deviation filter. So for example both 4 and "4" work.~~ EDIT: Deleted outdated info

x/wasm-storage/types/errors.go

x/wasm-storage/keeper/filter.go

x/wasm-storage/types/filters.go

x/wasm-storage/types/filters_util.go

x/wasm-storage/types/sigma.go

x/wasm-storage/types/filters.go

x/wasm-storage/keeper/abci.go

x/wasm-storage/types/events.go

hacheigriega added 2 commits July 9, 2024 12:41

fix: fix mode filter bug and refactor

7c432a0

style: spelling fix to pass lint

23eed91

hacheigriega commented Jul 9, 2024

View reviewed changes

refactor: refactoring filtering by adding interface

778ba84

hacheigriega changed the title ~~fix: Mode filter fix and refactor~~ feat: Refactor filtering and adding std dev filter Jul 9, 2024

refactor: add filter none

f43c95e

hacheigriega marked this pull request as draft July 9, 2024 21:55

hacheigriega added 6 commits July 9, 2024 19:42

refactor: remove constructor from interface

2236fe6

test: update unit test

ff655f4

fix: hex encode tally vm input and add tests

688fb37

feat: std dev filtering with refactoring and test

7ec582e

refactor: use of errors to clean filtering code

ee60b97

feat: add std dev filter support for uint32,uint64,int32,int64

0e0432c

hacheigriega marked this pull request as ready for review July 12, 2024 21:01

fix: fix lint and test

989b586

hacheigriega requested review from Thomasvdam and FranklinWaller July 15, 2024 13:24

Thomasvdam reviewed Jul 15, 2024

View reviewed changes

hacheigriega added 2 commits July 15, 2024 14:05

fix: improve error handling

d141f29

chore: remove unused package

642bd01

fix: fix filter logic involving corrupt reveals

52daa99

FranklinWaller requested changes Jul 16, 2024

View reviewed changes

hacheigriega added 5 commits July 16, 2024 12:35

fix: address comments and fix std dev filter bug

31ee4ac

refactor: refactor std dev filtering code

163fc93

refactor: add Sigma type

bb15b42

refactor: add HalfStepInt type and fix bugs

22a571d

chore: minor comment fix

22de69d

fix: add neg field to HalfStepInt

9cf7b9e

hacheigriega requested review from Thomasvdam and FranklinWaller July 17, 2024 19:58

hacheigriega added 4 commits July 17, 2024 23:49

chore: rephrase comments

4c0d98b

test: add fuzz test for std dev filter

48e7260

test: update tally vm unit test

528509f

chore: replace test tally wasm

695aea3

Thomasvdam reviewed Jul 19, 2024

View reviewed changes

chore: address comments

9ae58f8

fix: use json number for standard deviation parsing

482764d

hacheigriega force-pushed the hy/filter-refactor branch from 1938a02 to 482764d Compare July 21, 2024 13:43

mariocao reviewed Jul 22, 2024

View reviewed changes

x/wasm-storage/types/errors.go Outdated Show resolved Hide resolved

FranklinWaller reviewed Jul 22, 2024

View reviewed changes

feat: use standard json path and address review comments

d85092e

mariocao approved these changes Jul 23, 2024

View reviewed changes

Thomasvdam reviewed Jul 23, 2024

View reviewed changes

x/wasm-storage/keeper/abci.go Outdated Show resolved Hide resolved

feat: add tally exit code to tally completion event

bc8662a

hacheigriega requested a review from Thomasvdam July 23, 2024 13:41

Thomasvdam approved these changes Jul 23, 2024

View reviewed changes

Thomasvdam requested a review from FranklinWaller July 23, 2024 13:48

Thomasvdam reviewed Jul 23, 2024

View reviewed changes

x/wasm-storage/types/events.go Outdated Show resolved Hide resolved

chore: update event attribute name

4334f73

FranklinWaller approved these changes Jul 24, 2024

View reviewed changes

Thomasvdam mentioned this pull request Jul 24, 2024

✨ Support tally phase standard deviation filter sedaprotocol/seda-sdk#57

Closed

2 tasks

hacheigriega merged commit 4334f73 into main Jul 24, 2024
16 of 17 checks passed

hacheigriega deleted the hy/filter-refactor branch July 24, 2024 14:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Refactor filtering and adding std dev filter #300

feat: Refactor filtering and adding std dev filter #300

hacheigriega commented Jul 9, 2024 •

edited

Loading

hacheigriega Jul 9, 2024

Thomasvdam left a comment

hacheigriega commented Jul 15, 2024

hacheigriega commented Jul 15, 2024

FranklinWaller commented Jul 16, 2024

hacheigriega commented Jul 18, 2024

hacheigriega commented Jul 19, 2024

Thomasvdam commented Jul 19, 2024

hacheigriega commented Jul 19, 2024 •

edited

Loading

feat: Refactor filtering and adding std dev filter #300

feat: Refactor filtering and adding std dev filter #300

Conversation

hacheigriega commented Jul 9, 2024 • edited Loading

Explanation of Changes

hacheigriega Jul 9, 2024

Choose a reason for hiding this comment

Thomasvdam left a comment

Choose a reason for hiding this comment

hacheigriega commented Jul 15, 2024

hacheigriega commented Jul 15, 2024

FranklinWaller commented Jul 16, 2024

hacheigriega commented Jul 18, 2024

hacheigriega commented Jul 19, 2024

Thomasvdam commented Jul 19, 2024

hacheigriega commented Jul 19, 2024 • edited Loading

hacheigriega commented Jul 9, 2024 •

edited

Loading

hacheigriega commented Jul 19, 2024 •

edited

Loading