Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
docs(releases): add Stellar GMP and ITS release #523
base: main
Are you sure you want to change the base?
docs(releases): add Stellar GMP and ITS release #523
Changes from all commits
1ea0b4d
1e28779
473db2e
10ed0f0
25756e7
bc767c9
624268b
d19e65e
940ab13
bd0e433
27047b7
89a469d
e89219e
0578489
c69be06
3f3b020
3335c14
03d55f8
f510de8
0e33521
6676244
fa71cde
301000b
f05ab7f
f33bd92
1cd9572
29a450f
9eb8a86
a0fcffc
ef2fe64
7d7af00
0a4276c
7fa2c46
e2dcbc5
3c5c40d
7075eb6
2c608df
8c5ec6b
4458a30
b63627c
3602e91
90943b4
1906469
59a0371
9456db5
9ac56ae
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
@milapsheth should we capture
codeIDs
used per network to capture the deployment version in the doc? I understand it is already done in the config, but this will help ensure some parameters to be used during cosmwasm contract deployments.For instance, a release is needed in the wasm contracts to make stellar work, that way we should document the expected codeID to be available before we can deploy?
Alternatively, we can have a CI/CD check in place to make sure the deployed contract and the codeID mentioned are same and we do not depricated to an obsolete codeID.
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.
We can divide the prereqs in different MD files as per requirement.
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.
Decision behind the naming for other networks should be documented, since it differs from other chains.
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.
Should we mention gov address per chain here, since it is fixed per chain?
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.
admin address as well
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.
add a table with the addresses per network
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.
asked @eguajardo to see the CONTRACT_ADMIN is different based on network
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.
I believe these are the CONTRACT_ADMIN (Multisig Governance Key) on Gateway, VotingVerifier and MultisigProver
Stagnet:
axelar12qvsvse32cjyw60ztysd3v655aj5urqeup82ky
Testnet:
axelar12f2qn005d4vl03ssjq07quz6cja72w5ukuchv7
Mainnet:
axelar1nctnr9x0qexemeld5w7w752rmqdsqqv92dw9am
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.
No salt is specified in these commands and the contract admin should be picked up from the config, see here: https://www.notion.so/bright-ambert-2bd/Amplifier-SUI-Onboarding-509a654917aa4363857c7806fb98efa2?pvs=4#13dc53fccb7780cc9021e9f99c9c4936
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.
We will need salt in case we want to redeploy.
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.
And to generate same address on different networks, for testnet/mainnet.
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.
1 AXL is not sufficient for rewards. Maybe ask Ayush/Canh what value should be used for stagnet/testnet/mainnet. Rewards need to be topped up regularly. Can be updated in a follow-up PR, but let's not forget about it, cc @RiceAndMeet
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.
For testnetet, stagenet and devnet-amplifier, we add 100uaxl as rewards per epoch, and use 1 AXL to kickoff the reward pool, and it has worked so far. @milapsheth if you want we can increase the initial reward pool top up to 100 AXL, which should be enough to start. For mainnet, @canhtrinh will need to calculate based on the price of the asset being added on the week of deployment, so it would be best to leave it as TBD for
rewards_per_epoch
and rewards amount.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.
Why do we need it? Can we ignore
RUN_AS_ACCOUNT
as it is picked up as governance automatically? Not sure if this is needed on devnets.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.
add a table
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.
asked @eguajardo about the PROVER_ADMIN values
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.
Also see the Flow contract setting by network?
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.
Should add a query command to check proposal was successful.
For example:
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.
It would be best to move this step after all proposals pass, because we need to make an announcement to the verifiers to update their config.
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.
We should add ampd command to register here.
Also add a note to update infrastructure repo.
Look at the instructions here: https://www.notion.so/bright-ambert-2bd/Amplifier-SUI-Onboarding-509a654917aa4363857c7806fb98efa2?pvs=4#14ac53fccb77803aaf15dbdc390fe6f3
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.
mainnet is still missing
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.
Those values for Mainnet is TBD, as it needs to be checked with BD or Canh. https://www.notion.so/bright-ambert-2bd/Stellar-ITS-Release-doc-Review-Temp-196c53fccb77800f8865ca39c464e134?pvs=4#196c53fccb7780dd8aa8fa324a6e91e8 Should I ask BD or Canh about this value?
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.
You can ask Canh + Talal (when in doubt, ask both or whoever you think might know lol to unblock)
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.
query commands should be added for other proposals as well.
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.
participation threshold is off vs stagenet above. double check these values for flow on each network, you can see the values on different networks on axelarscan
https://devnet-amplifier.axelarscan.io/amplifier-rewards/flow
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.
I referenced values from the notion, @RiceAndMeet Could you double check those values?
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.
These values are based off configs from SUI multisig reward pools, which has different participation threshold compare to SUI voting verifier pool.
Can change these values to match flow reward pool configs if thats the correct reference.
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.
Not sure why we have these misc inconsistencies. Let's use Flow as reference, since the original params were decided for that, unless there's a good reason for an exception
cc @blockchainguyy
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.
https://www.notion.so/bright-ambert-2bd/Amplifier-Contract-Params-f7fc3401a2f040438c12ba50018383eb?pvs=4
@ahramy pls refer to the doc, some values can be removed from TBD.
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.
More funds need to be added later if we just start with 1 AXL
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.
@RiceAndMeet Could you double check this value?
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.
I figured we can start off with 1AXL on mainnet and add more funds later once we have clarity from BD.
For testnet/stagenet, I suppose we can do start off with more , maybe 100AXL.
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.
@RiceAndMeet Please get clarity from BD parallely, so we don't have issues later in the onboarding process