✨ Allow deleting contracts #800
Conversation
crates/contract-sdk/src/lib.rs
Outdated
| @@ -43,7 +43,7 @@ macro_rules! info { | |||
| } | |||
| } | |||
|
|
|||
| pub type RunResult<T> = Result<(String, T, Vec<RegisterContractEffect>), String>; | |||
| pub type RunResult<T> = Result<(String, T, Vec<RegisterContractEffect>, Vec<ContractName>), String>; | |||
There was a problem hiding this comment.
je trouve que ce type devient de moins en moins lisible... pas le sujet de refacto dans cette PR, mais faudra changer pour un truc un chouille plus "explicite", ou proposer un builder pattern "RunResultBuilder" avec des helpers explicite (e.g. .with_registered_contracts() ,.with_removed_contracts() ...)
There was a problem hiding this comment.
Bonne idée mais je ferais plus tard effectivement, en attendant je reswitch sur un seul Vec qui aidera
| }) | ||
| } | ||
|
|
||
| fn handle_delete_blob( |
There was a problem hiding this comment.
là j'ai l'impression que n'importe qui peut delete n'importe quel contrat non ? Comment fonctionne le "droit" de delete ?
| #[derive( | ||
| Debug, Serialize, Deserialize, Default, Clone, PartialEq, Eq, BorshSerialize, BorshDeserialize, | ||
| )] | ||
| pub struct DeleteContractAction { |
There was a problem hiding this comment.
un bout de RustDoc sur cette struct serait bien, pour avoir qqpart l'explication de comment on l'utilise / comment les "droits de suppression" sont gérés (s'ils le sont).
Peut-être de même avoir un bout de doc qqpart sur le register de contract pour expliquer comment on update un contrat ? (ok pour faire ça dans une autre PR)
There was a problem hiding this comment.
En soit ça c'est purement la spec du contrat hyle TLD, c'est réutilisable par d'autres mais pas forcément lié à comment l'utiliser.
Je vais quand même rajouter un peu de doc.
src/node_state.rs
Outdated
| contracts: &HashMap<ContractName, Contract>, | ||
| current_contracts: BTreeMap<ContractName, Contract>, | ||
| current_contracts: BTreeMap<ContractName, ContractSideEffect>, |
There was a problem hiding this comment.
note: je comprends pas bien la diff entre "contracts" et "current_contracts", à voir si on trouve un meilleur naming
There was a problem hiding this comment.
current_contracts c'est la version "en cours de settlement", tandis que contracts c'est la version actuellement storée dans node-state (et donc non mutable).
Le 'mieux' serait d'avoir une version Copy-on-write smart du truc. Peut ptet le wrapper dans un type en attendant.
src/node_state.rs
Outdated
| // Recursion end-case: we succesfully settled all prior blobs, so success. | ||
| let Some(current_blob) = blob_iter.next() else { | ||
| return Some((current_contracts, blob_proof_output_indices, true)); | ||
| return Some(Ok(SettlementResult { | ||
| contracts_changes: current_contracts, |
There was a problem hiding this comment.
en référence à mon commentaire précédent, peut-être que "contract_changes" est plus adapté comme nom de paramètre ?
src/node_state.rs
Outdated
| return match handle_blob_for_hyle_tld(contracts, ¤t_contracts, ¤t_blob.blob) | ||
| { | ||
| Ok(HyleTldOutput::Register(contract)) => { | ||
| let mut us = current_contracts.clone(); |
There was a problem hiding this comment.
chaud pour éviter les abbréviations comme "us" au maximum 🙏
There was a problem hiding this comment.
je pense que tu utilises des abbréviations pour limiter la taille de la ligne, mais je pense que c'est pas un bon pattern, et qu'on devrait plutôt chercher à moins nester nos fonctions, qu'à raccourcir le nom de nos variables ^^
There was a problem hiding this comment.
Non là c'est "us" parce que mmmh je sais plus si c'était pour updated_states ou si c'était us au sens ma version haha (genre us vs them comme les branches git)
wraitii
force-pushed
the
feat/delete_contracts
branch
2 times, most recently
from
c7a2ad5 to
61c71f0
Compare
wraitii
force-pushed
the
feat/delete_contracts
branch
from
7728b9f to
b64b196
Compare
No description provided.