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

feat: upgrade to ethers v6 #491

Merged
merged 3 commits into from
Apr 15, 2024
Merged

feat: upgrade to ethers v6 #491

merged 3 commits into from
Apr 15, 2024

Conversation

dtbuchholz
Copy link
Contributor

Summary

Bumps ethers to v6—plus, OpenZepplin and Hardhat packages that need v6 as a peer dependency.

Details

N/A

How it was tested

Tested deployment scripts:

  • Locally on hardhat node
  • On Seplolia here

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

})) as TablelandTables
).deployed();
console.log("New proxy address:", tables.address);
// @ts-expect-error ignore `Conversion of type 'Contract'` error since
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i tried to get around this error but couldn't. the type that deployProxy says it should return is an ethers Contract, but if you log it, it's a BaseContract—which is what TablelandTables extends. ignoring it resolved the issue.


// Check implementation
// Note: We poll here because the impl won't be visible from the proxy until the next tipset on Filecoin.
// Note: See https://docs.filecoin.io/smart-contracts/developing-contracts/best-practices/#consistently-generating-transaction-receipts.
const startTime = Date.now();
while (!(await checkImpl(tables.address))) {
while (!(await checkImpl(await tables.getAddress()))) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the new way to get an addr is via method call

const tableId = createEvent.args!.tableId;
console.log("Healthbot table created as:", `healthbot_${chainId}_${tableId}`);

// Insert first row into health bot table
const runStatement = `insert into healthbot_${chainId}_${tableId} values (1);`;
// Add 20% buffer to gas limit to avoid out-of-gas errors
Copy link
Contributor Author

Choose a reason for hiding this comment

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

txs were failing when i tried to deploy to a live network, i.e., sepolia. the gas limit was hitting 99%+. this adds a buffer and fixed the issue.

@@ -0,0 +1,5 @@
import type { EventLog, Log } from "ethers";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the response from receipt?.logs (see above) is now a Log or EventLog, so we need to make sure we only get the EventLog in order to parse

sanderpick
sanderpick previously approved these changes Apr 13, 2024
Copy link
Member

@sanderpick sanderpick left a comment

Choose a reason for hiding this comment

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

very nice!

@dtbuchholz
Copy link
Contributor Author

@sanderpick realized i didn't add tests here since i was rushing to test amoy support 😅 ...added. also, hardhat dropped node 16 support, so i updated the gh action to use node 18 (and technically, they say they only support node LTS).

@dtbuchholz dtbuchholz merged commit 542df81 into main Apr 15, 2024
2 checks passed
@dtbuchholz dtbuchholz deleted the dtb/ethers-v6 branch April 15, 2024 16:54
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.

2 participants