Skip to content

Conversation

@mmackz
Copy link
Contributor

@mmackz mmackz commented Oct 7, 2025

Description

  • filters decoded logs using the action step signature instead of hashing the AbiEvent object. This allows us to work with event signatures for events that do not have an available abi.

See screenshot for example. We can derive the types from this and create a pseudo abi, but when you use toEventSignature the hash is the result of the event name + the param types. Since we do not know the event name in this case we cannot derive the correct hash, but since we know what the hash is already (topic0) we can use it directly.

CleanShot 2025-10-06 at 20 56 26@2x

Summary by CodeRabbit

  • Bug Fixes

    • Improved event log filtering to match an action step’s provided signature, increasing accuracy and reducing false positives when events share selectors—validation and decoded results are more reliable.
  • Chores

    • Added a changeset declaring a patch release for the SDK with a note about the updated log filtering approach.

@changeset-bot
Copy link

changeset-bot bot commented Oct 7, 2025

🦋 Changeset detected

Latest commit: 0191f89

The changes in this PR will be included in the next version bump.

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

💥 An error occurred when fetching the changed packages and changesets in this PR
Some errors occurred when validating the changesets config:
The package or glob expression "@boostxyz/test" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.

@coderabbitai
Copy link

coderabbitai bot commented Oct 7, 2025

Walkthrough

Adds a changeset note declaring a patch release. Updates EventAction.isActionStepValid to filter event logs using the action step’s signature instead of computing the event selector. No public APIs or types changed.

Changes

Cohort / File(s) Summary
Release note
\.changeset/early-keys-sort.md
New changeset documenting a patch: “use actionStep signature to filter decoded logs.” No code changes.
SDK event log filtering
packages/sdk/src/Actions/EventAction.ts
In isActionStepValid for event-based steps, log filtering now matches log.topics[0] to the provided signature instead of toEventSelector(event). Decoding and validation flow unchanged.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller
  participant EventAction
  participant Provider as Chain Provider

  Caller->>EventAction: isActionStepValid(step, txReceipt)
  EventAction->>Provider: Retrieve logs from txReceipt
  Note over EventAction: NEW: Filter logs where topics[0] === step.signature
  alt Matching logs found
    EventAction->>EventAction: Decode matching logs
    EventAction->>Caller: Return validity result
  else No matches
    EventAction->>Caller: Return invalid (no matching event)
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my ears at topics’ trace,
A signature now sets the pace.
No selector chase, no needless jog—
I sniff the match in every log.
Patch packed tight, I thump with glee,
Hop, filter, validate—ship it, whee! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description provides a clear “### Description” section with detailed explanation and a screenshot, but it omits the required top-level guidelines link directing reviewers to CONTRIBUTING.md as well as the closing “💔 Thank you!” footer defined in the repository’s description template. Please add the “🚨 Please review the guidelines for contributing” line at the top of the description and include the closing “💔 Thank you!” footer so the PR fully conforms to the repository’s description template.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly summarizes the main change by indicating that decoded logs will now be filtered using the action step signature, which directly reflects the core modification in the code. It focuses on the primary update without extraneous details.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch matthew/compare-signature

Comment @coderabbitai help to get the list of available commands and usage tips.

@jonathandiep
Copy link
Contributor

Warnings
⚠️

By using the [ADHOC] title prefix, you are bypassing best practice protections.

Generated by 🚫 dangerJS against 62befe0

1 similar comment
@jonathandiep
Copy link
Contributor

Warnings
⚠️

By using the [ADHOC] title prefix, you are bypassing best practice protections.

Generated by 🚫 dangerJS against 62befe0

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/sdk/src/Actions/EventAction.ts (2)

616-616: Apply the same signature-based filtering for consistency.

This line still computes the event selector from the ABI using toEventSelector(event), but the PR's objective is to use the signature directly. The signature variable is already available at line 568, so this should be updated for consistency with the change at line 780.

Apply this diff:

-      decodedLogs = receipt.logs
-        .filter((log) => log.topics[0] === toEventSelector(event))
-        .map((log) => decodeAndReorderLogArgs(event, log));
+      decodedLogs = receipt.logs
+        .filter((log) => log.topics[0] === signature)
+        .map((log) => decodeAndReorderLogArgs(event, log));

1366-1366: Apply the same signature-based filtering for consistency.

This line still computes the event selector from the ABI using toEventSelector(event), but the PR's objective is to use the signature directly. The signature variable is already available at line 1323, so this should be updated for consistency with the change at line 780.

Apply this diff:

-    const decodedLogs = receipt.logs
-      .filter((log) => log.topics[0] === toEventSelector(event))
-      .map((log) => decodeAndReorderLogArgs(event, log));
+    const decodedLogs = receipt.logs
+      .filter((log) => log.topics[0] === signature)
+      .map((log) => decodeAndReorderLogArgs(event, log));
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b36f86d and 62befe0.

📒 Files selected for processing (2)
  • .changeset/early-keys-sort.md (1 hunks)
  • packages/sdk/src/Actions/EventAction.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Use Case Tests / Use Case Tests
  • GitHub Check: Verify / Verify
🔇 Additional comments (2)
.changeset/early-keys-sort.md (1)

1-5: LGTM!

The changeset correctly declares a patch release with an appropriate description of the change.

packages/sdk/src/Actions/EventAction.ts (1)

780-780: LGTM! Change aligns with PR objective.

Using the action step's signature directly instead of computing it from the ABI enables working with events that don't have an available ABI.

@jonathandiep
Copy link
Contributor

Warnings
⚠️

By using the [ADHOC] title prefix, you are bypassing best practice protections.

Generated by 🚫 dangerJS against 62befe0

@jonathandiep
Copy link
Contributor

Warnings
⚠️

By using the [ADHOC] title prefix, you are bypassing best practice protections.

Generated by 🚫 dangerJS against 0191f89

1 similar comment
@jonathandiep
Copy link
Contributor

Warnings
⚠️

By using the [ADHOC] title prefix, you are bypassing best practice protections.

Generated by 🚫 dangerJS against 0191f89

@mmackz mmackz merged commit cb705f0 into main Oct 7, 2025
7 checks passed
@mmackz mmackz deleted the matthew/compare-signature branch October 7, 2025 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants