refactor(types): vbyte -> storage score #1398
Conversation
sdk/src/types/block/output/rent.rs
Outdated
| @@ -31,145 +36,153 @@ const DEFAULT_BYTE_COST_FACTOR_BLOCK_ISSUER_KEY: u8 = 1; | |||
| serde(rename_all = "camelCase") | |||
| )] | |||
| pub struct RentStructure { | |||
There was a problem hiding this comment.
cli/src/command/account.rs
Outdated
| @@ -721,12 +721,12 @@ pub async fn send_native_token_command( | |||
| let address = address.convert()?; | |||
| let transaction = if gift_storage_deposit.unwrap_or(false) { | |||
| // Send native tokens together with the required storage deposit | |||
| let rent_structure = account.client().get_rent_structure().await?; | |||
| let rent_struct = account.client().get_rent_parameters().await?.into(); | |||
There was a problem hiding this comment.
no, it's converted into a RentStructure
There was a problem hiding this comment.
downstream effect ... RentStructure (new) wraps the RentParameters (old RentStructure) and a new field regarding implicit account creation. Unfortunately it is not reflected in the TIP yet, but here you can see that's it's going to be:
https://github.com/iotaledger/iota.go/pull/537/files#diff-74ba3861618d2996e83bd87e90b7259060214740d314c9d10cc0ae99c47df9c5R51-R55
This new field must be computed at runtime to create the new RentStructure instance, and this is what we have to use in the trait to calculate the score and hence the rent cost correctly.
There was a problem hiding this comment.
Since it's calculated at runtime, why not just make it a method on RentParameters?
There was a problem hiding this comment.
Aight, but even if it won't become part of the TIP, from a technical PoV I think it's still a good idea to calculate it only once at runtime (when the rent parameters are fetched from the node) and keep it buffered somewhere. I think it's fine to use a wrapper struct in order to keep RentParameters identical with the TIP. This is the path forward they chose in iota.go. One can go a different route, but you have to/should buffer that computed score, and not re-compute it unnecessarily.
There was a problem hiding this comment.
I think there's probably a better way to cache the value, if we need to
There was a problem hiding this comment.
Maybe, but since there's no immediately obvious way I would suggest to create an issue and someone from the team investigates a different approach in another/next sprint. I am strongly against caching the value globally or making everything overly complex just to prevent the wrapper. Wrappers are bread and butter in Rust so I don't really understand the discomfort with that 🤷
There was a problem hiding this comment.
I think there's no immediately obvious reason to cache this tbh
There was a problem hiding this comment.
hmm, I guess questioning that is fair! I'm changing it ...
|
Closing in favour of #1065 |
Description of change
Renttrait toStorageScoreand provides 2 fns:storage_scoreandrent_cost.RentStructuretoRentParametersand changes its semantics to offsets.RentStructuretype which wraps aRentParametersand something unmemorable but has to do with the implicit accountLinks to any relevant issues
Closes #1352
NOTE:
I turned this into a draft again, because the TIP isn't complete yet. So far this PR has implemented what was merged into
iota.gohere: iotaledger/iota.go#537 (except for the closures that can be provided on some storage score implementors)