Skip to content

Conversation

Le-Caignec
Copy link
Contributor

@Le-Caignec Le-Caignec commented Feb 17, 2025

No description provided.

@Le-Caignec Le-Caignec self-assigned this Feb 17, 2025
@Le-Caignec Le-Caignec changed the title Update changelog, package scripts, and migrate test files for IexecPo… feature/migrate-IexecPoco-to-ethers6 Feb 17, 2025
@Le-Caignec Le-Caignec marked this pull request as ready for review February 17, 2025 12:09
Le-Caignec and others added 2 commits February 18, 2025 19:13
Co-authored-by: Jérémy (James) Toussaint <33313130+jeremyjams@users.noreply.github.com>
iexecPoco,
[requester, sponsor, appProvider, datasetProvider],
[
0, // requester balance is unchanged, only frozen is changed
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can harmonize, 0 or 0n everywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some Number castings are still present in the code. Removing them in another PR might reveal the need to add explicit castings for some cases. I have only changed the ones that are actually consumed and where BigInt is required. I have minimized the changes as much as possible.

Le-Caignec and others added 4 commits February 19, 2025 21:27
…est.ts

Co-authored-by: Jérémy (James) Toussaint <33313130+jeremyjams@users.noreply.github.com>
…est.ts

Co-authored-by: Jérémy (James) Toussaint <33313130+jeremyjams@users.noreply.github.com>
Co-authored-by: Jérémy (James) Toussaint <33313130+jeremyjams@users.noreply.github.com>
Copy link
Contributor

@james-toussaint james-toussaint left a comment

Choose a reason for hiding this comment

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

Good job thanks 💪 !

Comment on lines +942 to +943
// const appOrderConsumedSlotIndex = ethers.keccak256(
// ethers.concat([
Copy link
Contributor

Choose a reason for hiding this comment

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

That's funny to update even the comments

Le-Caignec and others added 3 commits February 19, 2025 21:36
Co-authored-by: gfournieriExec <100280020+gfournieriExec@users.noreply.github.com>
requester: requester.address,
prices: ordersPrices,
volume,
trust: 3,
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't want to migrate the trust as well in bigint ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes it think but I have only changed lines/variables that are actually consumed and where BigInt is required. I have minimized the changes as much as possible.

Copy link
Contributor

@gfournieriExec gfournieriExec left a comment

Choose a reason for hiding this comment

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

Some minor comment but thanks huge work well done

@Le-Caignec Le-Caignec merged commit 01f2ac8 into feature/migration-to-ether-v6 Feb 19, 2025
1 check passed
@Le-Caignec Le-Caignec deleted the feature/migrate-IexecPoco-to-ethers6 branch February 19, 2025 15:34
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.

4 participants