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.

Sign up for GitHub

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

Jump to bottom

chore: deprecate network URLs, add asset ID and chain ID for mainnet #3107

Merged
merged 20 commits into from
Sep 11, 2024
Merged

chore: deprecate network URLs, add asset ID and chain ID for mainnet #3107

merged 20 commits into from
Sep 11, 2024

Conversation

Dhaiwat10
Copy link
Member

@Dhaiwat10 Dhaiwat10 commented Sep 5, 2024 •
edited
Loading

Release notes

In this release, we:

  • Deprecated the two network URLs, added asset ID and chain ID for mainnet

Summary

This PR deprecates the two network URLs currently exported by the SDK, and adds the asset ID and chain ID for mainnet.

The constants will eventually be deleted, so we have moved them inside of @internal/utils so that we can still use them in our tests.

Checklist

  • All changes are covered by tests (or not applicable)
  • All changes are documented (or not applicable)
  • I reviewed the entire PR myself (preferably, on GH UI)
  • I described all Breaking Changes (or there's none)

@Dhaiwat10 Dhaiwat10 added the chore Issue is a chore label Sep 5, 2024
@Dhaiwat10 Dhaiwat10 self-assigned this Sep 5, 2024
@vercel Vercel
Copy link

vercel bot commented Sep 5, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
create-fuels-template ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 10, 2024 10:15am
ts-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 10, 2024 10:15am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
create-fuels-counter-example ⬜️ Ignored (Inspect) Sep 10, 2024 10:15am

@Dhaiwat10 Dhaiwat10 changed the title chore: add url, asset ID and chain ID for mainnet chore: add network URL, asset ID and chain ID for mainnet Sep 5, 2024
@Dhaiwat10 Dhaiwat10 marked this pull request as ready for review September 5, 2024 05:47
danielbate
danielbate previously approved these changes Sep 5, 2024
View reviewed changes
Copy link
Contributor

@danielbate danielbate left a comment

Choose a reason for hiding this comment

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

Approving with comment 👍🏻

packages/account/src/providers/chains.ts Show resolved Hide resolved
@petertonysmith94
Copy link
Contributor

I haven't deprecated the old URLs like the original issue suggests because I think people will still need them, at least the testnet one for sure. Thoughts? cc @petertonysmith94 @arboleya

I personally believe deprecation would be the best way forward.

Users managing their own endpoints seems like a reasonable ask to me. Also removes coupling our SDK to specific endpoints.

arboleya
arboleya requested changes Sep 5, 2024
View reviewed changes
packages/account/src/configs.ts Outdated Show resolved Hide resolved
@Dhaiwat10 Dhaiwat10 changed the title chore: add network URL, asset ID and chain ID for mainnet chore: deprecate network URLs, add asset ID and chain ID for mainnet Sep 5, 2024
nedsalk
nedsalk previously approved these changes Sep 9, 2024
View reviewed changes
danielbate
danielbate previously approved these changes Sep 9, 2024
View reviewed changes
maschad
maschad reviewed Sep 9, 2024
View reviewed changes
Copy link
Member

@maschad maschad left a comment

Choose a reason for hiding this comment

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

LGTM but we should update the changeset as @petertonysmith94 suggested @Dhaiwat10

@Dhaiwat10 @petertonysmith94
update changeset
b58aa4f 
Co-authored-by: Peter Smith <peter@blueoceancomputing.co.uk>
@Dhaiwat10 Dhaiwat10 enabled auto-merge (squash) September 9, 2024 14:06
@Dhaiwat10 Dhaiwat10 requested review from xgreenx, petertonysmith94, maschad, danielbate and nedsalk and removed request for xgreenx September 9, 2024 14:06
maschad
maschad approved these changes Sep 9, 2024
View reviewed changes
Copy link
Member

@maschad maschad left a comment

Choose a reason for hiding this comment

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

Great job @Dhaiwat10

@github-actions GitHub Actions
Copy link
Contributor

Coverage Report:

Lines Branches Functions Statements
79.35%(-0.02%) 72.11%(-0.04%) 77.73%(+0%) 79.42%(-0.02%)
Changed Files:
Ok File (✨=New File) Lines Branches Functions Statements
🔴 ✨ internal/utils/src/constants.ts 0%
(+0%)		 100%
(+100%)		 100%
(+100%)		 0%
(+0%)
🔴 packages/account/src/providers/transaction-request/transaction-request.ts 88.57%
(+0%)		 76.71%
(-1.37%)		 84%
(+0%)		 88.81%
(+0%)

@Dhaiwat10 Dhaiwat10 merged commit c98461f into master Sep 11, 2024
23 checks passed
@Dhaiwat10 Dhaiwat10 deleted the dp/mainnet-url-assets branch September 11, 2024 02:45
@fuel-service-user fuel-service-user mentioned this pull request Sep 11, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arboleya arboleya arboleya left review comments

@maschad maschad maschad approved these changes

@danielbate danielbate danielbate approved these changes

@petertonysmith94 petertonysmith94 petertonysmith94 approved these changes

@Torres-ssf Torres-ssf Torres-ssf approved these changes

@xgreenx xgreenx xgreenx left review comments

@digorithm digorithm Awaiting requested review from digorithm digorithm is a code owner

@nedsalk nedsalk Awaiting requested review from nedsalk nedsalk is a code owner

Assignees

@Dhaiwat10 Dhaiwat10

Labels
chore Issue is a chore
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add ChainID and AssetID for mainnet
8 participants
@Dhaiwat10 @petertonysmith94 @arboleya @maschad @danielbate @nedsalk @xgreenx @Torres-ssf