search

fennelLabs / Fennel-Protocol

The Unlicense
5 stars 4 forks source link

Identity Bounds Checking #61

Closed Romulus10 closed 2 years ago

Romulus10 commented 2 years ago

Add a check for an invalid identity index. For instance, if someone tries calling an extrinsic with an identity index greater than the IdentityNumber, we can short-circuit and throw an error "Invalid Identity Index" or "Identity does not exist".

let identity_number = <IdentityNumber<T>>::get(); //the .get() is already called in every extrinsic
ensure!(identity_id < identity_number, Error::<T>::IdentityDoesNotExist); // new piece of code
Romulus10 commented 2 years ago

@isaacadams So Ed and I just looked through the identity pallet - where did you see this coming in to play?

isaacadams commented 2 years ago

Anyone can call an extrinsic... right? They can input any number into the extrinsic calls that ask for an identity index. That is where it comes into play for extrinsic calls which alter an existing identity.

Romulus10 commented 2 years ago

Most of the extrinsics check first if the identity requested is owned by the sender. Wouldn't that throw out any invalid identity IDs? I see how it would come into play during revocation though.

Romulus10 commented 2 years ago

Actually, I don't think it's even needed there, is it? If the identity can't be removed nothing is committed to chain state anyway.

isaacadams commented 2 years ago

Yes, that is correct. This suggestion isn't a "saving our bacon" situation. It is a short-circuit / efficiency suggestion.

Doesn't it cost more weight to read from storage? If the identity index is invalid, why go through the trouble of indexing the IdentityList if we can quickly check the validity of the identity index and return the error.