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

Abstract L2 #5446

Merged
merged 1 commit into from
Feb 17, 2025
Merged

Abstract L2 #5446

merged 1 commit into from
Feb 17, 2025

Conversation

samholmes
Copy link
Contributor

@samholmes samholmes commented Feb 7, 2025

  • Fix yarn.lock
  • Add abstract core plugin

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

Dependencies

none

Requirements

If you have made any visual changes to the GUI. Make sure you have:

  • Tested on iOS device
  • Tested on Android device
  • Tested on small-screen device (iPod Touch)
  • Tested on large-screen device (tablet)

@samholmes samholmes marked this pull request as draft February 7, 2025 00:56
Copy link
Contributor

@swansontec swansontec left a comment

Choose a reason for hiding this comment

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

The chainCode seems wrong. Also, please drop the first commit, or replace it with the upgrade to edge-currency-accountbased.

@@ -183,6 +183,19 @@ export const getSpecialCurrencyInfo = (pluginId: string): SpecialCurrencyInfo =>
export const SPECIAL_CURRENCY_INFO: {
[pluginId: string]: SpecialCurrencyInfo
} = {
abstract: {
allowZeroTx: true,
chainCode: 'ETH',
Copy link
Contributor

Choose a reason for hiding this comment

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

This is used for FIO. Does FIO support Abstract? If so, is this the code they use?

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 don't know if FIO supports Abstract or not. I copied the chainCode from all the other EVM based SpecialCurrencyInfos. Do these other configs need fixing too?

Copy link
Contributor

Choose a reason for hiding this comment

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

Most likely you have found a bug!

@samholmes samholmes marked this pull request as ready for review February 17, 2025 23:47
@samholmes samholmes enabled auto-merge February 17, 2025 23:47
@samholmes samholmes merged commit a11d45e into develop Feb 17, 2025
2 checks passed
@samholmes samholmes deleted the sam/abstract branch February 17, 2025 23:58
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