Migrate IexecAccessors tests to ethers v6 - #189
Merged
james-toussaint merged 7 commits intoFeb 14, 2025
Merged
Conversation
| expect(task.status).to.equal(TaskStatusEnum.ACTIVE); | ||
| expect(task.dealid).to.equal(dealId); | ||
| expect(task.idx).to.equal(taskIndex); | ||
| expect(task.timeref).to.equal(timeRef); |
Contributor
There was a problem hiding this comment.
why remove that ?
Contributor
Author
There was a problem hiding this comment.
Good catch, some old testing, updated with Re-add timeref check, thanks 👍
Le-Caignec
approved these changes
Feb 13, 2025
| return ethers.utils._TypedDataEncoder.hashDomain({ | ||
| //TODO: Move to utils | ||
| export async function hashDomain(domain: IexecLibOrders_v5.EIP712DomainStructOutput) { | ||
| return ethers.TypedDataEncoder.hashDomain({ |
Contributor
There was a problem hiding this comment.
Suggested change
| return ethers.TypedDataEncoder.hashDomain({ | |
| return TypedDataEncoder.hashDomain({ |
other option: directly import TypedDataEncoder module
Contributor
Author
There was a problem hiding this comment.
Sure see Import from ethers (same comment everywhere)
Comment on lines
+571
to
+595
| function extractEvents( | ||
| logs: Log[], | ||
| address: string, | ||
| eventName: string, | ||
| eventAbi: string[], | ||
| ): LogDescription[] { | ||
| const eventInterface = new ethers.Interface(eventAbi); | ||
| const event = eventInterface.getEvent(eventName); | ||
| if (!event) { | ||
| throw new Error('Event name and abi mismatch'); | ||
| } | ||
| return ''; | ||
| const eventId = event.topicHash; | ||
| let extractedEvents = logs | ||
| // Get logs of the target contract. | ||
| .filter((log) => log.address === address && log.topics.includes(eventId)) | ||
| // Parse logs to events. | ||
| .map((log) => eventInterface.parseLog(log)) | ||
| // Get events with the target name. | ||
| .filter((event) => event && event.name === eventName); | ||
| // Get only non null elements. | ||
| // Note: using .filter(...) returns (LogDescription | null)[] | ||
| // which is not desired. | ||
| const events: LogDescription[] = []; | ||
| extractedEvents.forEach((element) => element && events.push(element)); | ||
| return events; |
Contributor
There was a problem hiding this comment.
An other way to do that, that we use in SDK
export const getEventFromLogs = ({ contract, eventName, logs }) => {
const filter = contract.getEvent(eventName);
if (!filter) {
throw new Error(`Event filter not found for ${eventName}`);
}
const eventTopic = filter.fragment.topicHash;
// Find the event in the logs based on the topic
const eventFound = logs.find((log) => log.topics[0] === eventTopic);
if (!eventFound) {
throw new Error(`Event ${eventName} not found in logs`);
}
// Check if the event is already decoded
if (eventFound.args) {
return eventFound;
}
// If not decoded, decode the event
try {
return {
...eventFound,
args: contract.interface.decodeEventLog(
filter.fragment,
eventFound.data,
eventFound.topics
),
};
} catch (error) {
throw new Error(`Failed to decode event ${eventName}: ${error.message}`);
}
};
Contributor
There was a problem hiding this comment.
In fact log.topics.includes(eventId) could be replace by log.topics[0] === eventTopic. Indeed this is the first element of the topic that correspond to the event signature.
Other improvements could be considered, such as removing the need to use the address.
Contributor
Author
There was a problem hiding this comment.
zguesmi
reviewed
Feb 13, 2025
Base automatically changed from
feature/hardhat-and-ethers-upgrade-part1
to
feature/migration-to-ether-v6
February 13, 2025 16:58
Co-authored-by: Zied Guesmi <[email protected]>
zguesmi
approved these changes
Feb 14, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See job logs on
Run teststep ✔️ :