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.

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

itests: Add tests for EthGetLogs filter combinations #10052

Merged
merged 10 commits into from
Jan 20, 2023

Conversation

iand
Copy link
Contributor

@iand iand commented Jan 18, 2023

Related Issues

Part of #9849

Proposed Changes

  • Refactors EthFilter itests to simplify and remove repeated code
  • Adds a new solidity contract (EventMatrix) for testing all the filter combinations
  • Add TestEthGetLogs with subtests exercising filter combinations:
    • one address, two alternate addresses
    • one topic, one topic with alternate values (OR)
    • two topics with single and alternate values (AND then OR)
  • Add TestEthGetLogsWithBlockRanges with subtests testing various block ranges
    • before a block height
    • after block height
    • within block height range
    • outside range
    • latest/earliest keywords
  • Fixes a double decoding of cbor (but Logs: Indexing/Filtering bugs ref-fvm#1345 will remove the original decoding) - currently we are consistent, but we will move the decoding boundary so that cbor encoded values become the internal format to be decoded when emitted at the api level (currently they are decoded when being ingested)
  • Fixes a parsing error of topic values from JSON that prevented OR conditions

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

@iand iand mentioned this pull request Jan 18, 2023
42 tasks
@iand iand changed the title Add itest for EthGetLogs filter combinations itests: Add tests for EthGetLogs filter combinations Jan 19, 2023
@iand iand marked this pull request as ready for review January 19, 2023 14:22
@iand iand requested a review from a team as a code owner January 19, 2023 14:22
@iand iand added the FEVM label Jan 19, 2023
@travisperson travisperson self-requested a review January 19, 2023 17:22
Comment on lines 1912 to 1914
if orig == nil {
return orig
}
Copy link
Member

Choose a reason for hiding this comment

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

len(orig) == 0 is safer.

Comment on lines +103 to +110
decodedEntries := make([]types.EventEntry, len(ev.Entries))
for i, entry := range ev.Entries {
decodedEntries[i] = types.EventEntry{
Flags: entry.Flags,
Key: entry.Key,
Value: decodeLogBytes(entry.Value),
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this go in the opposite direction than Steb pointed out in filecoin-project/ref-fvm#1345 (1)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants