-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
chore: update internally used chain config #3420
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Dhaiwat10 will the differences between the local
and ignition
chains cause issues here? Consensus parameters version for local
is V2
and for ignition
(and other configs) it is V1
.
@petertonysmith94 That's a good point. I'm not really sure which one we are supposed to use. I heard from one of the team members that apparently the local config specified by them hasn't been updated in a long time. I'm not sure why the local is Gonna try a few things and get back with some more context for our sync. |
It seems our local test chain config uses settings from the Testnet upgrade number 6 Maybe we should go for either the latest Testnet upgrade (upgrade number 9) or even the lastest Mainet Upgrade Both of them seem to be using the This will require changes only on our side if I am not mistaken, and only on TS types related to test helpers. It seems that even though these versions are being updated on the chain config JSON for main-net and test-net, fuel-core always resolves them to |
… into dp/update-chain-config
@Torres-ssf can you check the changes I made and if they make sense? |
CodSpeed Performance ReportMerging #3420 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, we should wait to merge this until the breaking change window.
At the very least, we should add release notes and a detailed summary around this - as it could affect the end consumer and should have migration notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@petertonysmith94 happy to wait if their are concerns, but I'd personally prioritise parity with mainnet. I assume consumers aren't using our exported chain config for testing as they are seeing different results in mainnet.
But @petertonysmith94, this PR does not introduce breaking changes. |
@danielbate I do agree we should prioritise parity with mainnet - hence the approval.
@Torres-ssf For me, the state config changes and, thereby, the changes to wallets with funds in both the fuels CLI and testing infrastructure are breaking changes. I believe I'm just being overly cautious, though. I think it's worth adding a summary about:
# Before all these addresses had funds
0x09c0b2d1a486c439a87bcba6b46a7a1a23f3897cc83a94521a96da5c23bc58db
0x09dd7a49174d6fcc9f4c6f7942c18060a935ddd03ee69b594189b8c3581276ea
0x1c31df52b6df56407dd95f83082e8beb9cfc9532ac111d5bd8491651d95ba775
0x3b24509ed4ab3c7959f5c9391c1445c59290cdb5f13d6f780922f376b7029f30
0x49075a7538e2c88ebe1926ce4d898198a2a4e790d14512943a9864bc536b3c82
0x5d99ee966b42cd8fc7bdd1364b389153a9e78b42b7d4a691470674e817888d4e
0x6a2c4691c547c43924650dbd30620b184b5fe3fb6dbe5c4446110b08f6f405bf
0x77c6f40b7da70d885f68efaad7c661327482a63ea10dcb4271de819438254ae1
0x7e3626e306588eba79cafab73f0709e55ab8f4bdfe8c8b75034a430fc56ece89
0x86604282dc604481b809845be49667607c470644f6822fc01eb0d22f167e08cf
0x94ffcc53b892684acefaebc8a3d4a595e528a8cf664eeb3ef36f1020b0809d0d
0x95a7aa6cc32743f8706c40ef49a7423b47da763bb4bbc055b1f07254dc729036
0xb32197cf75efe05bf453c26178139f09b391582065549c1422bc92555ecffb64
0xbca334a06d19db5041c78fe2f465b07be5bec828f38b7796b2877e7d1542c950
0xbd9a1dc8d3ec3521c43f6c2c01611b4d0204c7610204ff0178488c8738a30bd2
0xbdaad6a89e073e177895b3e5a9ccd15806749eda134a6438dae32fc5b6601f3f
0xcee104acd38b940c8f1c62c6d7ea00a0ad2241d6dee0509a4bf27297508870d3 # Now only these addresses are pre-funded
0x09c0b2d1a486c439a87bcba6b46a7a1a23f3897cc83a94521a96da5c23bc58db
0x94ffcc53b892684acefaebc8a3d4a595e528a8cf664eeb3ef36f1020b0809d0d |
@petertonysmith94 happy to wait until after the code freeze. Does that sound good? |
I'm happy for it to go out, just good to have the PR description detailing the change and release notes |
@petertonysmith94 does the description look good now? Feel free to edit further if you want to :) |
Coverage Report:
Changed Files:
|
Just missing release notes :) |
@petertonysmith94 A summary suffices I think, no need for release notes on this one. Remember that all PRs are still mentioned and listed on the release's changelog. The release notes are just a premium spot to advertise more relevant things. |
I trust your judgment on this one. |
Removed release notes |
stateConfig.json
#2783Summary
This PR updates our internally used chain config to match the one specified here and removes all the state from our default
stateConfig.json
files.Things to note:
consensus_parameters
have been updated fromV1
toV2
in the chain config files.create fuels
andfuels CLI
:Checklist