feat: charge fee for signature verification - #1651
Conversation
| ExecutionEvent::PaidFromAssetLockDriveEvent { | ||
| identity, | ||
| operations, | ||
| signature_verifications, |
There was a problem hiding this comment.
No, this should be something like "validation_costs"
There was a problem hiding this comment.
Fixed. Renamed to validation_operations
| // Apply signature verification costs | ||
| if let Some(signature_verifications) = signature_verifications { | ||
| let total_verification_cost: Credits = | ||
| signature_verifications.iter().fold(0, |acc, op| { |
There was a problem hiding this comment.
you need to handle errors, I don't think fold is right.
| identity: PartialIdentity, | ||
| /// the operations that the identity is requesting to perform | ||
| operations: Vec<DriveOperation<'a>>, | ||
| signature_verifications: Option<Vec<SignatureVerificationOperation>>, |
There was a problem hiding this comment.
They should not be an Option.
There was a problem hiding this comment.
This should contain a vec of validation Operations, which should be an enum
| identity: Option<&PartialIdentity>, | ||
| ) -> Option<Vec<SignatureVerificationOperation>> { | ||
| match state_transition { | ||
| StateTransition::IdentityCreate(state_transition) => Some( |
There was a problem hiding this comment.
There's also an hashed ecdsa verification
| let key = identity | ||
| .loaded_public_keys | ||
| .get(&state_transition.signature_public_key_id()) | ||
| .unwrap(); |
There was a problem hiding this comment.
don't unwrap in production code.
There was a problem hiding this comment.
Do we really need to introduce Result to this function?
We are expecting signer identity to posses signature_public_key_id. If it doesn't, then something radically wrong is happening.
| let key = identity | ||
| .loaded_public_keys | ||
| .get(&state_transition.signature_public_key_id().unwrap()) | ||
| .unwrap(); |
There was a problem hiding this comment.
don't unwrap in production
| for validation_operation in validation_operations { | ||
| let cost = validation_operation.processing_cost(platform_version)?; | ||
| individual_fee_result | ||
| .checked_add_assign(FeeResult::default_with_fees(0, cost)) |
There was a problem hiding this comment.
Not ideal to do it this way as checked_add_assign is more much expensive than adding, imo it would be best to add all costs for processing, then only do 1 checked_add_assign.
| pub trait OperationLike { | ||
| fn processing_cost(&self, platform_version: &PlatformVersion) -> Result<Credits, Error>; | ||
|
|
||
| fn storage_cost(&self, platform_version: &PlatformVersion) -> Result<Credits, Error>; |
There was a problem hiding this comment.
I don't think a validation operation would ever have a storage_cost.
| let key = identity | ||
| .loaded_public_keys | ||
| .get(&state_transition.signature_public_key_id()) | ||
| .unwrap(); |
There was a problem hiding this comment.
Don't unwrap here, have the function return an error.
| StateTransition::IdentityUpdate(state_transition) => { | ||
| let mut operations = vec![]; | ||
|
|
||
| let identity = identity.unwrap(); |
| let key = identity | ||
| .loaded_public_keys | ||
| .get(&state_transition.signature_public_key_id().unwrap()) | ||
| .unwrap(); |
| pub fn signature_verification_operations_from_state_transition( | ||
| state_transition: &StateTransition, | ||
| identity: Option<&PartialIdentity>, | ||
| ) -> Vec<ValidationOperation> { |
There was a problem hiding this comment.
I believe the better approach is to get the operations from the execution context.
There was a problem hiding this comment.
Also this was not versioned
|
Closed in favor of #1656 |
3 participants
Issue being fixed or feature implemented
Feature implements fee charging for signature verification in state transitions.
What was done?
Added validation operations for every state transitions:
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only