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: bic-333_mee_support #166

Merged
merged 14 commits into from
Jan 16, 2025
Merged

feat: bic-333_mee_support #166

merged 14 commits into from
Jan 16, 2025

Conversation

joepegler
Copy link
Collaborator

@joepegler joepegler commented Jan 13, 2025

PR-Codex overview

This PR focuses on the rebranding of AbstractJS, introduces support for meeNode, and enhances the SDK with new functionalities and constants. It also updates various components to improve the overall structure and usability of the codebase.

Detailed summary

  • Deleted getCounterFactualAddress.ts.
  • Updated tokens/index.ts to export __AUTO_GENERATED__.
  • Added support for mee in clients/decorators/index.ts.
  • Introduced new instructions in account/decorators/instructions/index.ts.
  • Updated CHANGELOG.md for version 0.0.29.
  • Modified style settings in biome.json.
  • Changed ALT_CHAIN_ID to MAINNET_CHAIN_ID in workflows/unit-tests.yml.
  • Added useTestBundler option throughout various client and decorator files.
  • Introduced new types and functions in account/utils/Types.ts and account/utils/contractSimulation.ts.
  • Enhanced testing files with new functionality and improved existing tests.
  • Updated README.md to reflect SDK name change to abstractJS.

The following files were skipped due to too many changes: src/sdk/account/decorators/buildBridgeInstructions.test.ts, src/sdk/account/utils/explorer.test.ts, src/test/testUtils.ts, src/sdk/clients/decorators/mee/executeSignedQuote.test.ts, src/sdk/clients/decorators/mee/getQuote.test.ts, src/sdk/account/decorators/build.test.ts, src/sdk/clients/createHttpClient.ts, src/sdk/account/utils/getMultichainContract.test.ts, src/sdk/clients/decorators/mee/signFusionQuote.test.ts, src/sdk/clients/decorators/mee/signFusionQuote.ts, src/sdk/account/decorators/instructions/buildIntent.ts, src/sdk/clients/decorators/mee/waitForSupertransactionReceipt.ts, src/sdk/account/decorators/build.ts, src/sdk/account/decorators/getFactoryData.test.ts, src/sdk/account/decorators/getUnifiedERC20Balance.ts, src/sdk/account/decorators/getNexusAddress.ts, src/sdk/account/toMultiChainNexusAccount.test.ts, src/sdk/account/decorators/queryBridge.ts, src/sdk/constants/abi/AccountFactory.ts, src/sdk/account/toNexusAccount.addresses.test.ts, src/sdk/account/toNexusAccount.ts, src/sdk/account/decorators/getFactoryData.ts, src/sdk/clients/decorators/mee/index.ts, src/sdk/account/utils/toAcrossPlugin.ts, src/sdk/clients/createMeeClient.test.ts, src/sdk/account/toMultiChainNexusAccount.ts, scripts/fetch:tokenMap.ts, src/sdk/account/utils/getMultichainContract.ts, src/sdk/clients/decorators/mee/getQuote.ts, src/sdk/account/decorators/buildBridgeInstructions.ts, src/sdk/constants/tokens/__AUTO_GENERATED__.ts

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Copy link

github-actions bot commented Jan 13, 2025

size-limit report 📦

Path Size
core (esm) 36.35 KB (0%)
core (cjs) 45.72 KB (0%)
bundler (tree-shaking) 523 B (0%)
paymaster (tree-shaking) 543 B (0%)

@joepegler joepegler marked this pull request as draft January 14, 2025 10:57
@joepegler joepegler force-pushed the feat/add_mee branch 2 times, most recently from 0dd7c8c to 743196e Compare January 14, 2025 17:26
@joepegler joepegler force-pushed the feat/add_mee branch 3 times, most recently from 6b89cac to 1d6ac35 Compare January 14, 2025 21:55
@joepegler joepegler marked this pull request as ready for review January 15, 2025 11:33
@joepegler joepegler changed the title feat/bic-333_mee_support feat: bic-333_mee_support Jan 15, 2025
// First instruction: Bridge USDC tokens
mcNexus.buildInstructions({
action: {
type: "BRIDGE",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not the biggest fan of using the string as a selector here.

const intent = await prepareIntent({
  account: mcNexus,
  amount: parseUnits('100', 6),
  toChain: optimism,
  token: mcUSDC
})

Maybe an extensions model would be better. As I do believe we'll be developing quite a few plugins and we'd like to enable others to do so as well.

Idea:

const mcNexus = await toMultichainNexusAccount({
  chains: [optimism, base, polygon],
  signer: eoa,
  extensions: [
    intentActions
  ]
})

const intent = await mcNexus.buildIntent({
  amount: parseUnits('100', 6),
  toChain: optimism,
  token: mcUSDC
})
``

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmmm, I like the intstruction -> intent terminology change, but the "BRIDGE" here is how we differentiate between the different types of intents, because we want each of them to be build from the same generic helper (prepareIntent being generic helper in this case). Would you prefer enums over strings here? I prefer strings because users don't have to import the enum from our package. Typing the string is more straightforward...

Does this work for you @TurboFolkBlackMetal?

const intent = await mcNexus.buildIntent({
  type: "BRIDGE",
  amount: parseUnits('100', 6),
  toChain: optimism,
  token: mcUSDC
})

Copy link
Collaborator

Choose a reason for hiding this comment

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

An Intent is just one type of Instruction, actually prepareIntent would return Instruction[].

It has specific terminology in the space where intent means - "move tokens from multiple chains to one or more chains".

Best if we go through some things on a short call imo

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For me semantically I prefer this (1):

const instructions: Instruction[] = [
  buildInstruction({ type: "intent", params: ... }),
  buildInstruction({ type: "anotherInstructionType", params: ... })
]

over this (2):

const instructions: Instruction[] = [
  buildIntent({ params: ... }),
  buildAnotherInstructionType({ params: "" })
]

Because in option 1 it's clear that each of the elements in the array resolve to the same thing (an instruction), and I know immediately know about the only multipurpose helper which generates instructions. With option two if I don't know what helpers are available I'd need to visit the docs to investigate each of the different types of instruction helpers I can use. With option 1 I can use my IDE and know everything I need to know about building every instruction type by clicking this:
image
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I want to be guided by typescript in my IDE over visiting docs, especially given our troubled history of keeping documentation up to date.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will get us to a happy medium so that we have the benefit of these helpers being ‘modularised’ & ‘standalone’ helpers in their own directory with custom names, but I will still access them from this ‘buildInstructions’ helper. That work for you?

@joepegler joepegler force-pushed the feat/add_mee branch 3 times, most recently from e5ef20c to bf56eac Compare January 15, 2025 21:45
@joepegler joepegler force-pushed the feat/add_mee branch 2 times, most recently from 7911577 to 5baa467 Compare January 16, 2025 11:55
@joepegler joepegler force-pushed the feat/add_mee branch 3 times, most recently from 52a3cce to 9b9cf7d Compare January 16, 2025 16:47
@joepegler joepegler merged commit a48f556 into develop Jan 16, 2025
5 checks passed
@joepegler joepegler deleted the feat/add_mee branch January 16, 2025 17:06
joepegler added a commit that referenced this pull request Jan 22, 2025
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