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

migrate to ethers v6 #2374

Open
wants to merge 74 commits into
base: feat/major-poc
Choose a base branch
from
Open

Conversation

zaidarain1
Copy link
Contributor

No description provided.

Copy link

nx-cloud bot commented Nov 13, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 015ec60. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

@zaidarain1 zaidarain1 force-pushed the feat/ethers-v6-migration branch 3 times, most recently from d00fbd7 to 1c0a592 Compare November 26, 2024 23:56
@zaidarain1 zaidarain1 force-pushed the feat/ethers-v6-migration branch from a6c281e to bdafd2d Compare November 27, 2024 06:17
@zaidarain1 zaidarain1 marked this pull request as ready for review November 28, 2024 01:57
@zaidarain1 zaidarain1 requested review from a team as code owners November 28, 2024 01:57
@zaidarain1 zaidarain1 requested review from shineli1984 and a team as code owners November 29, 2024 06:13
@zaidarain1 zaidarain1 force-pushed the feat/ethers-v6-migration branch 2 times, most recently from cb7c27d to c0d1cfb Compare December 5, 2024 05:04
@zaidarain1 zaidarain1 force-pushed the feat/ethers-v6-migration branch 4 times, most recently from b094201 to 2d08363 Compare December 6, 2024 00:03
Copy link
Contributor

@Sam-Jeston Sam-Jeston left a comment

Choose a reason for hiding this comment

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

The orderbook updates make sense to me. One nit around remaining v5 naming that doesnt apply anymore

callerAddress: string,
): TransactionBuilder {
return async () => {
const v6ContractTransaction = await transactionMethods.buildTransaction();

const v5PopulatedTransaction: PopulatedTransaction = {
const v5PopulatedTransaction: PreparedTransactionRequest = {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this is no longer a v5 transaction mapping layer, so a rename through here would be clearer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cheers, addressed this issue 😄

@zaidarain1 zaidarain1 force-pushed the feat/ethers-v6-migration branch from f4ab878 to 140adfc Compare December 9, 2024 03:09
@zaidarain1 zaidarain1 force-pushed the feat/ethers-v6-migration branch from 140adfc to d321490 Compare December 9, 2024 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants