search

GenesysGo / shadow-drive-rust

Apache License 2.0
21 stars 13 forks source link

Commitment Level #6

Closed cavemanloverboy closed 2 years ago

cavemanloverboy commented 2 years ago

Hey there, y'all. Been trying out the rust sdk. Love it so far. Will be submitting issues for separate bits of feedback. Number 1:

Would be great if we could change the commitment status ever-where to be max confirmations -- or to perhaps make it an option when initializing a ShadowDriveClient with a default of max confirmation. i.e. ::new(wallet, rpc_client) returns a client set to max confirmations and ::new_with_commitment(.. , .. Commitment) returns a client set to the specified commitment level.

This is an issue because when I tried to test the sdk I wasn't able to validate that my specified changes were actually applied:

Testing add storage
previous size: 11000000
txn id: "4euLrqYYqmgqVycSinVxohRmKvhXVFAPqnSvZU4bkLH9sAHftoZW4Nau3azgrcCu8iaCJk6UcSvHEjtN8356E9Tp"
new size: 11000000

Testing reduce storage
previous size: 11000000
txn id: "539D51aFXfjJBZL1zi375VCqfPUK2gm9PzPKs1L4AnsDSMmkmdynhkwfeWodJB44edGw6oTJiEjDDtiUVtX4Dbjn"
new size: 11000000
cavemanloverboy commented 2 years ago

This includes e.g. changing

    pub async fn get_storage_account(&self, key: &Pubkey) -> ShadowDriveResult<StorageAccount> {
        let account_info = self.rpc_client.get_account(&key)?;
        StorageAccount::try_deserialize(&mut account_info.data.as_slice())
            .map_err(Error::AnchorError)
    }

to use self.rpc_client.get_account_with_commitment https://docs.rs/solana-client/1.7.11/solana_client/rpc_client/struct.RpcClient.html#method.get_account_with_commitment

VegetarianOrc commented 2 years ago

Sounds great. I'll make the change tonight. Leaning toward the defaulting to max confirmations and allowing override to use at your own risk.

VegetarianOrc commented 2 years ago

@cavemanloverboy I've upped the version to 0.2.0 to reflect the changing of the constructor based on your feedback. Please feel free to check out PR #8 .

I'll likely wait to re-publish to crates.io until we've worked in more feedback to capture any breaking API changes in one go.

VegetarianOrc commented 2 years ago

Gonna merge & publish. Will just bump the version number if there's another breaking change.