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

Fix TBRv3 library issues #104

Merged
merged 35 commits into from
Jan 22, 2025
Merged

Fix TBRv3 library issues #104

merged 35 commits into from
Jan 22, 2025

Conversation

Alex99y
Copy link
Collaborator

@Alex99y Alex99y commented Dec 27, 2024

  • Update all Wormhole SDK ts libs to the latest version (v1.3.0) and add all of them as a peerDependency to avoid the collision of multiple Wormhole SDK versions in one project.
  • Fix the layout for Solana devnet SC to deserialize a key that only exists in that network
  • Remove the mint address HTTP request and pass the data via function args.
  • Pass parameter value unwrapIntent for EVM
  • Fix amount parsing for Solana gasDropOff
  • Update @solana/web3.js dependency to the latest version (^1.98.0)
  • Upgrade yarn version from 4.4.1 to 4.6.0

@Alex99y Alex99y changed the title Fix TBRv3 Fix TBRv3 library issues Dec 30, 2024
sdk/solana/tbrv3/token-bridge-relayer.ts Outdated Show resolved Hide resolved
sdk/solana/tbrv3/token-bridge-relayer.ts Show resolved Hide resolved
Comment on lines +77 to +78
const allowedTokenAddresses = this.config?.allowedTokenAddresses || [];
const isWhitelistConfigAvailable = this.config && allowedTokenAddresses.length > 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

What's all this about? This does not exactly fit the theme of the repo name of "arbitrary-token-transfers", no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@nonergodic It is funny because I asked the same question. They want this restriction to prevent users from bridging useless tokens. By default is inactive and I believe that this feature will only be used in Portal Bridge UI.
@solanoepalacio can give you more default about this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It sounds to me that a deny list would be more desirable in that case 🤔. But I'm not sure. I'll ask.

Copy link
Collaborator

Choose a reason for hiding this comment

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

A few notes that shed a -little- bit of light to this.
There's two limitations that prevent to fully embracing the "arbitrary token transfers" nature of this protocol:

  • At the UI level the user doesn't have the ability to just select "any" token. Connect is configured with a set of supported tokens at bootstrap. I think @adam and @Art (from foundation and whlabs respectively) are working on adding the ability to enter a token address. Probably they are doing it directly against the token bridge, but we'd need to check.
  • To fully support "arbitrary-token-transfers" with automatic relaying we are faced with the problem of needing attestations in the target chain. IOW: While the relayer can't attest a token that hasn't been transferred before to a chain, we can't offer automatic ATT to users because probably the relay will fail. (There a ton of considerations on the UX side that make this implementation trickier than one may think).

For this reason, there's a "phased" plan to get to Automatic Token Transfers. Plan is like so IIRC:

  1. Release TBRv3 with a whitelist of the 30 most used tokens in each chains (this PR)
  2. Make portal support any token that has already been attested
  3. Add automatic attestation on the relayer and make the final release of Automatic Token Transfers (with automatic relaying)

connect/platforms/solana/src/automatic.ts Outdated Show resolved Hide resolved
sdk/solana/tbrv3/token-bridge-relayer.ts Show resolved Hide resolved
deployment/package.json Show resolved Hide resolved
connect/platforms/solana/src/automatic.ts Outdated Show resolved Hide resolved
Comment on lines +77 to +78
const allowedTokenAddresses = this.config?.allowedTokenAddresses || [];
const isWhitelistConfigAvailable = this.config && allowedTokenAddresses.length > 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It sounds to me that a deny list would be more desirable in that case 🤔. But I'm not sure. I'll ask.

@scnale scnale dismissed feli-xlabs’s stale review January 22, 2025 17:14

All requests were addressed.

@scnale scnale merged commit e76f129 into main Jan 22, 2025
1 check passed
@scnale scnale deleted the fix/TBRv3 branch January 22, 2025 17:31
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.

5 participants