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!: updated chain assets, removed beta-5 network #2432

Merged
merged 13 commits into from
Jun 11, 2024

Conversation

petertonysmith94
Copy link
Contributor

@petertonysmith94 petertonysmith94 commented May 31, 2024

Closes #2281

Breaking change

  • Chain ID entry for beta5 has been removed, suggest to use testnet.
    // Before
    import { CHAIN_IDS } from 'fuels';
    const beta5 = CHAIN_IDS.fuel.beta5;
    
    // After
    import { CHAIN_IDS } from 'fuels';
    const testnet = CHAIN_IDS.fuel.testnet;

@petertonysmith94 petertonysmith94 added the bug Issue is a bug label May 31, 2024
@petertonysmith94 petertonysmith94 self-assigned this May 31, 2024
@petertonysmith94 petertonysmith94 marked this pull request as ready for review May 31, 2024 17:12
@petertonysmith94 petertonysmith94 changed the title fix: incorrect assets fix!: updated chain assets, removed beta-5 network May 31, 2024
@petertonysmith94
Copy link
Contributor Author

I had to remove beta-5 as all the chain ID's were 0 (beta-5, devnet and testnet), causing issues with resolving the correct assets.

@fuel-service-user
Copy link
Contributor

fuel-service-user commented May 31, 2024

✨ A PR has been created under the rc-2432 tag on fuels-wallet repo.
FuelLabs/fuels-wallet#1329

@LuizAsFight
Copy link
Contributor

instead of using a static assets.json , could we have a script that would run post-build ? something like

const devnetProvider = await Provider.create(DEVNET_URL);
const devnetChainId = provider.getChainId();
const devnetBaseAssetId = provider.getBaseAssetId();

const testnetProvider = await Provider.create(TESTNET_URL);
const testnetChainId = provider.getChainId();
const testnetBaseAssetId = provider.getBaseAssetId();

const assets = {
..... // builds the asset structure we have
};

fs.write('assets.json', assets);

@Torres-ssf
Copy link
Contributor

instead of using a static assets.json , could we have a script that would run post-build ? something like

const devnetProvider = await Provider.create(DEVNET_URL);
const devnetChainId = provider.getChainId();
const devnetBaseAssetId = provider.getBaseAssetId();

const testnetProvider = await Provider.create(TESTNET_URL);
const testnetChainId = provider.getChainId();
const testnetBaseAssetId = provider.getBaseAssetId();

const assets = {
..... // builds the asset structure we have
};

fs.write('assets.json', assets);

This would indeed be a good solution for this problem

@petertonysmith94 petertonysmith94 marked this pull request as draft June 10, 2024 08:59
@petertonysmith94 petertonysmith94 force-pushed the ps/fix/incorrect-assets branch from 47e2d53 to 65beef7 Compare June 10, 2024 14:00
@petertonysmith94
Copy link
Contributor Author

instead of using a static assets.json , could we have a script that would run post-build ? something like

const devnetProvider = await Provider.create(DEVNET_URL);
const devnetChainId = provider.getChainId();
const devnetBaseAssetId = provider.getBaseAssetId();

const testnetProvider = await Provider.create(TESTNET_URL);
const testnetChainId = provider.getChainId();
const testnetBaseAssetId = provider.getBaseAssetId();

const assets = {
..... // builds the asset structure we have
};

fs.write('assets.json', assets);

I explored this option and came to conclusion that it might not be the ideal solution.

  1. Makes our build dependent on the network (and therefore having an internet connection).
  2. We currently export assets and CHAIN_IDS from our fuel package.
  3. By only writing to assets.json, we could make the above exports out of sync.

I'm favouring pushing the hard-coded asset ID's for now and have added an e2e-assets test to ensure these values don't go out of sync with the networks.

@petertonysmith94 petertonysmith94 marked this pull request as ready for review June 10, 2024 14:13
Copy link
Contributor

Coverage Report:

Lines Branches Functions Statements
45.44%(+0.03%) 38.52%(+0.06%) 42.24%(+0.05%) 45.21%(+0.03%)
Changed Files:

Coverage values did not change👌.

@petertonysmith94 petertonysmith94 merged commit 41dc617 into master Jun 11, 2024
20 checks passed
@petertonysmith94 petertonysmith94 deleted the ps/fix/incorrect-assets branch June 11, 2024 16:19
@petertonysmith94 petertonysmith94 added this to the 0.x mainnet milestone Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect asset for devnet
7 participants