Closed tserg closed 6 months ago
LGTM so far. A valuable addition 👏
I'm thinking frontend_data_provider
should be upgradable, just in case. If we find a bug or sth, we won't have to update the frontend as well, just the contract itself.
Marking this for review first. I will run the Sepolia script after review and update the state file.
This was how I approached the re-deployment on Sepolia:
sncast
could be submitting the transactions in parallel causing some nonces to clash.Some tips for re-deployment in the future:
The deployment script failed many times because of a transaction error that says invalid nonce.
Experienced the same, see here for more details if you're curious.
First, I deleted the state file and executed the deployment script. As expected, it fails because some of the contracts have been declared.
Hmm, this could be good feedback / feature request for SN Foundry. It should be able to recover for an already declared class no? If the declare tx is not in the state file, it could just add it and continue, since a class is supposed to be declared only once. Do you mind opening an issue in the starknet-foundry repo to start the discussion? I think other teams would benefit from this as well.
Also, it's weird that there was a Shrine class clash on declare 🤔 According to the docs, a declare tx consists of a class hash which in turn is a hash of external entry points and other things. Since we added 2 new public fns to the Shrine, it should have a different hash, hence the declare tx should've succeeded.
Hmm, this could be good feedback / feature request for SN Foundry. It should be able to recover for an already declared class no? If the declare tx is not in the state file, it could just add it and continue, since a class is supposed to be declared only once. Do you mind opening an issue in the starknet-foundry repo to start the discussion? I think other teams would benefit from this as well.
Sure! tracking here
Also, it's weird that there was a Shrine class clash on declare 🤔 According to the docs, a declare tx consists of a class hash which in turn is a hash of external entry points and other things. Since we added 2 new public fns to the Shrine, it should have a different hash, hence the declare tx should've succeeded.
Yea, this is odd. I may also have fumbled on this since there were a couple of issues and I was focused on having everything deployed successfully rather than figuring out the exact behaviour of sncast, so I ended up applying fixes wholesale :sweat_smile:
I went through the Figma and tried to group the information required on one screen together into its own data type:
get_trove_assets_info(trove_id)
should provide all the necessary data for each collateral, with the information for each collateral asset in aTroveAssetInfo
struct. The CASH price at the top and the "Capacity" field can be obtained by callingget_yin_info()
. The "Target C-ratio" can be obtained by callingget_recovery_mode_info()
.ShrineAssetInfo
struct that is a member ofTroveAssetInfo
, but which might be useful for landing pages that give an overview of the protocol without displaying a trove in particular. This can be obtained by callingget_shrine_assets_info()
.Other changes
u32
) and the Sentinel (u64
). This is standardized tou32
which should be more than sufficient.