Add isFromSelf filter #88
Conversation
- Add isFromSelf() method to FilterAPI as alias for fromSelf() - Add async isFrom(address) method to filter by Ethereum address - Update filterMessages to support both sync and async filter callbacks - Implement address resolution using AddressResolver with caching - Add comprehensive test coverage for both new methods - Export AddressResolver from @hybrd/xmtp package - Update README with new filter methods and async usage example
Original prompt from Ian2. Implement the MethodsIn the filterAPI wrapper implementation within the same file: For
For
3. Design Decisions to MakeYou'll need to decide on:
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Deploying hybrid with
|
| Latest commit: |
d5755cd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8b3dfb95.hybrid-2zz.pages.dev |
| Branch Preview URL: | https://devin-hybrid-57-1759706919.hybrid-2zz.pages.dev |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="packages/core/src/behaviors/filter-messages.ts">
<violation number="1" location="packages/core/src/behaviors/filter-messages.ts:104">
A new `AddressResolver` instance is created on every `isFrom()` call. Because the resolver's cache is instance-based, this pattern defeats the caching mechanism, causing significant performance degradation by making expensive address resolution calls for every message instead of using the cache.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| context.message.senderInboxId === context.client.inboxId, | ||
| isFrom: async (address: `0x${string}`) => { | ||
| const normalizedAddress = address.toLowerCase() | ||
| const resolver = new AddressResolver(context.client) |
There was a problem hiding this comment.
A new AddressResolver instance is created on every isFrom() call. Because the resolver's cache is instance-based, this pattern defeats the caching mechanism, causing significant performance degradation by making expensive address resolution calls for every message instead of using the cache.
Prompt for AI agents
Address the following comment on packages/core/src/behaviors/filter-messages.ts at line 104:
<comment>A new `AddressResolver` instance is created on every `isFrom()` call. Because the resolver's cache is instance-based, this pattern defeats the caching mechanism, causing significant performance degradation by making expensive address resolution calls for every message instead of using the cache.</comment>
<file context>
@@ -94,11 +96,39 @@ export function filterMessages(
+ context.message.senderInboxId === context.client.inboxId,
+ isFrom: async (address: `0x${string}`) => {
+ const normalizedAddress = address.toLowerCase()
+ const resolver = new AddressResolver(context.client)
+
+ try {
</file context>
✅ Addressed in 02bf5c6
Move AddressResolver instantiation outside filterAPI to enable cache reuse across multiple isFrom() filter calls within the same context, improving performance as recommended by code review.
| ) | ||
|
|
||
| // Create AddressResolver instance for reuse across multiple isFrom() calls | ||
| const resolver = new AddressResolver(context.client) |
There was a problem hiding this comment.
Wait, why do we need to resolve an address when its a 0xstring? Address resolver is only for ENS names
|
You're right to question this! Let me clarify what's happening here: The Problem
What AddressResolver Does HereThe // We have: message.senderInboxId (e.g., "inbox-abc123")
// We need: sender's Ethereum address (e.g., "0x1234...")
const senderAddress = await resolver.resolveAddress(
context.message.senderInboxId, // XMTP inbox ID
context.conversation.id
)
// Now compare: senderAddress === inputAddressLooking at the Alternative ApproachesIf there's a better/simpler way to get the sender's address directly from the message or context without using AddressResolver, I'd be happy to refactor! For example:
Let me know if you'd prefer a different approach! |
|
I figure we should tackle these two tickets at the same time: https://linear.app/01studio/issue/HYBRID-59/add-sender-and-subjects-back-to-runtime Can you add sender and subjects back to the runtime? This should make it so we don't have to resolve sender for filterMessages only, we can do this higher up the chain and not have to check multiple times. |
- Uncommented sender and subjects fields in AgentRuntime interface - Added resolveSender() helper function in XMTP plugin to resolve sender addresses - Integrated address resolution using AddressResolver and BasenameResolver - Added subjects extraction from message content using extractSubjects() - Updated all 4 message handler locations to populate runtime.sender and runtime.subjects - Simplified isFrom() filter to use runtime.sender.address directly - Updated tests to mock runtime.sender and runtime.subjects - Removed redundant AddressResolver instantiation from filter-messages behavior This resolves the performance concern by resolving addresses once at the runtime level rather than repeatedly in filter methods. Fixes HYBRID-59 Relates to HYBRID-57
Resolved merge conflict in packages/xmtp/src/plugin.ts by accepting origin/main's restructured event handler architecture which already includes the HYBRID-59 sender/subjects resolution logic.
|
Great question! The current implementation (as of commit Here's what changed: Old Implementation (what you commented on)// Created AddressResolver instance for each filter check
const resolver = new AddressResolver(context.client)
isFrom: async (address: `0x${string}`) => {
const senderAddress = await resolver.resolveAddress(
context.message.senderInboxId, // ❌ Resolving inbox ID to address
context.conversation.id
)
return senderAddress?.toLowerCase() === normalizedAddress
}New Implementation (HYBRID-59)// No AddressResolver in filter-messages.ts at all!
isFrom: async (address: `0x${string}`) => {
const normalizedAddress = address.toLowerCase()
const senderAddress = context.runtime.sender?.address?.toLowerCase()
return senderAddress === normalizedAddress // ✅ Direct comparison
}What Happened?Your feedback prompted HYBRID-59, which added
This eliminates the redundant AddressResolver calls and addresses exactly what you requested! The AddressResolver is now only used at the plugin level, not in filters. Summary: The |
packages/xmtp/src/plugin.ts
Outdated
| * @param client - XMTP client instance | ||
| * @returns XmtpSender object with address, inboxId, name, and optional basename | ||
| */ | ||
| async function resolveSender( |
There was a problem hiding this comment.
Just use the Resolver router top level class. It exposes helper functions for each of the resolver. Don't use individual resolvers.
packages/xmtp/src/plugin.ts
Outdated
| } | ||
| ] | ||
|
|
||
| const basenameResolver = new BasenameResolver({ |
There was a problem hiding this comment.
Same here, use top level resolver router.
…lvers - Replace individual resolver imports with top-level Resolver class - Remove custom resolveSender() function, use Resolver.createXmtpSender() - Update extractSubjects() to accept Resolver instance - Use Resolver.resolveName() for universal name resolution - Create single Resolver instance shared across all event handlers - Fix test mock to include required senderInboxId field Addresses code review feedback from Ian on PR #88
ian
changed the title
ian
changed the title
Summary
Adds two new filter methods for message filtering by sender identity:
filter.isFromSelf()- Checks if message is from the agent itself