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

💡 [REQUEST] - Add fragment support for filterEventLogs #774

Closed
ifavo opened this issue Apr 4, 2024 · 7 comments · Fixed by #878
Closed

💡 [REQUEST] - Add fragment support for filterEventLogs #774

ifavo opened this issue Apr 4, 2024 · 7 comments · Fixed by #878

Comments

@ifavo
Copy link
Contributor

ifavo commented Apr 4, 2024

Summary

logs.filterEventLogs requires manually encoding the log filter, which is handled within other parts of the SDK already by supporting fragments.

The example code right now is:

// create encoder helper to handle event encoding
const event = new abi.Event(
  'event Transfer(address indexed from, address indexed to, uint256 amount)'
);
const encodedTopics = event.encodeFilterTopics([
  // first indexed value is "from", set to null to not use it
  null,

  // second indexed value is "to"
  '0x0000000000000000000000000000456e65726779',
]);

// access logs for a contract with filters
const filteredLogs = await thor.logs.filterEventLogs({
  criteriaSet: [
    // filter by address and topics, empty topics are ignored
    {
      address: '0x0000000000000000000000000000456e65726779',
      topic0: encodedTopics[0],
      topic1: encodedTopics[1],
      topic2: encodedTopics[2],
      topic3: encodedTopics[3],
      topic4: encodedTopics[4],
    },
  ]
});

Basic Example

Support fragments, the sample snippet above could be reduced to:

await thor.logs.filterEventLogs(
    criteriaSet: [
        {
            address: '0x0000000000000000000000000000456e65726779',
            fragment: 'event Transfer(address indexed from, address indexed to, uint256 amount)',
            args: ['0xd8da6bf26964af9d7eed9e03e53415d37aa96045', '0xa5cc3c03994db5b0d9a5eedd10cabab0813678ac']
        }
    ]
})

Its basically what already happens within getEventSubscriptionUrl:

// If the event is an ethers' EventFragment, format it to a 'full' event string
if (vechain_sdk_core_ethers.EventFragment.isFragment(event)) {
event = event.format('full');
}
const ev = new abi.Event(event);
// Encode the indexed parameters to construct the topic filters
const encodedTopics = ev.encodeFilterTopics(indexedValues ?? []);
return thorest.subscriptions.get.EVENT(baseURL, {
position: options?.blockID,
contractAddress: options?.address,
topic0: encodedTopics[0],
topic1: encodedTopics[1],
topic2: encodedTopics[2],
topic3: encodedTopics[3],
topic4: encodedTopics[4]
});

@Valazan
Copy link
Contributor

Valazan commented Apr 4, 2024

Hi @ifavo, even in this case, you can access the filter from the contract object and the topics will be encoded:

const contract = thorSoloClient.contracts.load(
       CONTRACT_ADDRESS,
       CONTRACT_ABI
       );
        
await contract.filters .Transfer(FROM_ADDRESS).get() // pass the criterias as arguments

@ifavo
Copy link
Contributor Author

ifavo commented Apr 4, 2024

@Valazan that's great to know! thank you 🙏

With the function filterEventLogs still differs from the other function that support fragments, I believe it should be in there too.

With your example I can query for a single event but can not filter for multiples in the same request, like I am able to do on the node-API.

@Valazan
Copy link
Contributor

Valazan commented Apr 4, 2024

@ifavo yes, currently multiple events are not supported.

I'll create an issue to implement your suggested call to enable the direct use of fragments with encoded args and the filtering of multiple events.

@Valazan
Copy link
Contributor

Valazan commented Apr 4, 2024

@ifavo I'm adding to the planning also the possibility to build a criteria from a contract object.

So this call

await thor.logs.filterEventLogs(
    criteriaSet: [
        {
            address: '0x0000000000000000000000000000456e65726779',
            fragment: 'event Transfer(address indexed from, address indexed to, uint256 amount)',
            args: ['0xd8da6bf26964af9d7eed9e03e53415d37aa96045', '0xa5cc3c03994db5b0d9a5eedd10cabab0813678ac']
        }
    ]
})

Could also be invoked like this:

await thor.logs.filterEventLogs(
    criteriaSet: [
        contract.criteria.Transfer('0xd8da6bf26964af9d7eed9e03e53415d37aa96045', '0xa5cc3c03994db5b0d9a5eedd10cabab0813678ac')
    ]
})

@nwbrettski
Copy link
Collaborator

Modify the current thor.logs.filterEventLogs to support the feature

Events will be filtered in both these ways, by either using a fragment or the contract object:

await thor.logs.filterEventLogs(
    criteriaSet: [
        {
            address: '0x0000000000000000000000000000456e65726779',
            fragment: 'event Transfer(address indexed from, address indexed to, uint256 amount)',
            args: ['0xd8da6bf26964af9d7eed9e03e53415d37aa96045', '0xa5cc3c03994db5b0d9a5eedd10cabab0813678ac']
        }
    ]
})

Could also be invoked like this:

await thor.logs.filterEventLogs(
    criteriaSet: [
        contract.criteria.Transfer('0xd8da6bf26964af9d7eed9e03e53415d37aa96045', '0xa5cc3c03994db5b0d9a5eedd10cabab0813678ac')
    ]
})

@ifavo
Copy link
Contributor Author

ifavo commented Apr 9, 2024

@Valazan @nwbrettski May I suggest to also decode the data before returning it?

The same is already happening on calling contract functions, the returned data is decoded. Asking the user to manually decode them for events adds, in my opinion, an unnecessary burden. I can separate this into a new issue if you like.

@ifavo
Copy link
Contributor Author

ifavo commented May 31, 2024

@Valazan thank you for making the improvement 🙌

I have a question regarding compatibility.

Syntax

As far as I understand you changed it from:

const filteredLogs = await thor.logs.filterEventLogs({
   criteriaSet: EventCriteria[]
})

to:

const filteredLogs = await thor.logs.filterEventLogs({
   criteriaSet: {
      criteria: EventCriteria
      eventFragment: EventFragment
   }[]
})

Why not keep it compatible with previous code by accepting both versions?
Due this change my code breaks when updating the SDK.

const filteredLogs = await thor.logs.filterEventLogs({
   criteriaSet: (EventCriteria | EventFragment)[]
})

Topic Hash now required

Filtering just by address, without hash worked before and is also supported by the node, it fails now with Error: Cannot read properties of undefined (reading 'topicHash'). Example:

const logs = await thor.logs.filterEventLogs({
  options: {
    offset: 0,
    limit: 3,
  },
  criteriaSet: [
    {
      criteria: {
        address: '0x0000000000000000000000000000456e65726779',
      },
    },
  ],
  order: 'asc',
});

Is this a wanted change?

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

Successfully merging a pull request may close this issue.

4 participants