Ruleset Unit Tests and tweaks #445
Conversation
🦋 Changeset detectedLatest commit: fb0810f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
panieldark
force-pushed
the
rulesets-tests-and-tweaks
branch
from
c8dd157 to
2b2b783
Compare
| | (typeof REJECT_ADDRESSES)[keyof typeof REJECT_ADDRESSES] | ||
| | (typeof APPROVE_ADDRESSES)[keyof typeof APPROVE_ADDRESSES]; | ||
|
|
||
| const MOCK_API_CALLS = apiCallsSnapshot as Record< |
There was a problem hiding this comment.
I know you guys hate these types, but it allows RuleSet.check() to allow for looser types being imported from json (ex: we recognize riskType with strict types, riskType: "COUNTERPARTY" | "OWNERSHIP" | "INDIRECT", but from json, riskType is typed as string, and thus it gives a TypeError, without these types
| return partial.threshold(data); | ||
| }) | ||
| ); | ||
| const shouldApplyRule = results[this.predicateFn]((_) => _); |
There was a problem hiding this comment.
Do we need ((_) => _)? Can we not just say results[this.predicateFn]()?
There was a problem hiding this comment.
We do; the predicate fns require a fn to predicate off of, and unfortunately in TypeScript there isn't cleaner syntactic sugar
| async check(deposit: ScreeningDepositRequest): Promise<Rejection | Delay> { | ||
| async check( | ||
| deposit: ScreeningDepositRequest, | ||
| cache: Partial<Record<ApiCallKeys, CallReturnData>> = {} |
There was a problem hiding this comment.
Can we alias the cache type? The really long types are unweildy, aliases would at least help a little bit
There was a problem hiding this comment.
Definitely, changed
| ): Promise<MisttrackAddressOverviewData> => { | ||
| const token = "ETH"; |
There was a problem hiding this comment.
Just a heads up (no action item rn), we may have to screen on more than one chain because sometimes if hack happens on optimism and is bridged back to ETH, the ETH screening call doesn't always show the hack. Made ticket
| TC_7: "0xadd7885af8f37df5c965e5d16caf16f807dc79a0", | ||
| } as const; | ||
|
|
||
| export const APPROVE_ADDRESSES = { |
There was a problem hiding this comment.
Ishaan is a good example to keep in here, he's a normal defi user but somehow gets flagged by TRM for stuff
There was a problem hiding this comment.
Sure thing, what is his address? I'll add him in a later PR addressing the tickets you made
Motivation
RuleSet work needs tests, what currently exists are only scripts
Solution
Adds tests, also tweaks a couple things about how RuleSets are defined
Proof
PR Checklist