Browse Source

Bug FSignatureChecked

Use FSignatureChecked is not a good solution because can cause to store
invalid signs. This is to undo 2.1.6 use of this field. Must ALLWAYS
check if signature is valid or not, because the Public key can be
changed between last FSignatureChecked and current safebox state
PascalCoin 7 years ago
parent
commit
c88c1c5485
2 changed files with 22 additions and 66 deletions
  1. 0 6
      Units/PascalCoin/UBlockChain.pas
  2. 22 60
      Units/PascalCoin/UOpTransaction.pas

+ 0 - 6
Units/PascalCoin/UBlockChain.pas

@@ -155,8 +155,6 @@ Type
   Private
     Ftag: integer;
   Protected
-    FSignatureChecked : Boolean; // Improvement TPCOperation speed 2.1.6
-    //
     FPrevious_Signer_updated_block: Cardinal;
     FPrevious_Destination_updated_block : Cardinal;
     FPrevious_Seller_updated_block : Cardinal;
@@ -1818,8 +1816,6 @@ begin
     P^.Op.FPrevious_Signer_updated_block := op.Previous_Signer_updated_block;
     P^.Op.FPrevious_Destination_updated_block := op.FPrevious_Destination_updated_block;
     P^.Op.FPrevious_Seller_updated_block := op.FPrevious_Seller_updated_block;
-    P^.Op.FHasValidSignature:=op.FHasValidSignature;
-    P^.Op.FSignatureChecked:=op.FSignatureChecked;
     h := op.Sha256;
     P^.Op.FBufferedSha256:=op.FBufferedSha256;
     P^.Op.tag := list.Count;
@@ -2105,7 +2101,6 @@ end;
 
 constructor TPCOperation.Create;
 begin
-  FSignatureChecked := False;
   FHasValidSignature := False;
   FBufferedSha256:='';
   InitializeData;
@@ -2188,7 +2183,6 @@ begin
   FPrevious_Seller_updated_block := 0;
   FHasValidSignature := false;
   FBufferedSha256:='';
-  FSignatureChecked := False;
 end;
 
 function TPCOperation.LoadFromNettransfer(Stream: TStream): Boolean;

+ 22 - 60
Units/PascalCoin/UOpTransaction.pas

@@ -509,19 +509,12 @@ begin
     end;
   end;
 
-  If Not FSignatureChecked then begin
-    If Not TCrypto.ECDSAVerify(account_signer.accountInfo.accountkey,GetOperationHashToSign(FData),FData.sign) then begin
-      errors := 'Invalid sign';
-      FHasValidSignature := false;
-      exit;
-    end else FHasValidSignature := true;
-    FSignatureChecked:=True;
-  end else begin
-    If Not FHasValidSignature then begin
-      errors := 'Invalid sign';
-      exit;
-    end;
-  end;
+  If Not TCrypto.ECDSAVerify(account_signer.accountInfo.accountkey,GetOperationHashToSign(FData),FData.sign) then begin
+    errors := 'Invalid sign';
+    FHasValidSignature := false;
+    exit;
+  end else FHasValidSignature := true;
+
   FPrevious_Signer_updated_block := account_signer.updated_block;
   FPrevious_Destination_updated_block := account_target.updated_block;
   If (public_key in FData.changes_type) then begin
@@ -609,7 +602,6 @@ begin
   end else begin
     FHasValidSignature := true;
   end;
-  FSignatureChecked:=True;
 end;
 
 function TOpChangeAccountInfo.toString: String;
@@ -660,7 +652,6 @@ begin
   end else begin
     FHasValidSignature := true;
   end;
-  FSignatureChecked:=True;
 end;
 
 function TOpTransaction.DoOperation(AccountTransaction : TPCSafeBoxTransaction; var errors : AnsiString): Boolean;
@@ -738,20 +729,12 @@ begin
   end;
 
   // Check signature
-  If Not FSignatureChecked then begin
-    _h := GetTransactionHashToSign(FData);
-    if (Not TCrypto.ECDSAVerify(sender.accountInfo.accountkey,_h,FData.sign)) then begin
-      errors := 'Invalid sign';
-      FHasValidSignature := false;
-      Exit;
-    end else FHasValidSignature := true;
-    FSignatureChecked:=True;
-  end else begin
-    If Not FHasValidSignature then begin
-      errors := 'Invalid sign';
-      exit;
-    end;
-  end;
+  _h := GetTransactionHashToSign(FData);
+  if (Not TCrypto.ECDSAVerify(sender.accountInfo.accountkey,_h,FData.sign)) then begin
+    errors := 'Invalid sign';
+    FHasValidSignature := false;
+    Exit;
+  end else FHasValidSignature := true;
   //
   FPrevious_Signer_updated_block := sender.updated_block;
   FPrevious_Destination_updated_block := target.updated_block;
@@ -1086,7 +1069,6 @@ begin
     TLog.NewLog(lterror,Classname,'Error signing a new Change key');
     FHasValidSignature := false;
   end else FHasValidSignature := true;
-  FSignatureChecked:=True;
 end;
 
 function TOpChangeKey.DoOperation(AccountTransaction : TPCSafeBoxTransaction; var errors: AnsiString): Boolean;
@@ -1170,19 +1152,11 @@ begin
     end;
   end;
 
-  If Not FSignatureChecked then begin
-    If Not TCrypto.ECDSAVerify(account_signer.accountInfo.accountkey,GetOperationHashToSign(FData),FData.sign) then begin
-      errors := 'Invalid sign';
-      FHasValidSignature := false;
-      exit;
-    end else FHasValidSignature := true;
-    FSignatureChecked:=True;
-  end else begin
-    If Not FHasValidSignature then begin
-      errors := 'Invalid sign';
-      exit;
-    end;
-  end;
+  If Not TCrypto.ECDSAVerify(account_signer.accountInfo.accountkey,GetOperationHashToSign(FData),FData.sign) then begin
+    errors := 'Invalid sign';
+    FHasValidSignature := false;
+    exit;
+  end else FHasValidSignature := true;
 
   FPrevious_Signer_updated_block := account_signer.updated_block;
   FPrevious_Destination_updated_block := account_target.updated_block;
@@ -1394,7 +1368,6 @@ begin
   FData.n_operation := n_operation;
   FData.fee := fee;
   FHasValidSignature := true; // Recover founds doesn't need a signature
-  FSignatureChecked := True;
 end;
 
 function TOpRecoverFounds.DoOperation(AccountTransaction : TPCSafeBoxTransaction; var errors: AnsiString): Boolean;
@@ -1624,19 +1597,11 @@ begin
     exit;
   end;
 
-  If Not FSignatureChecked then begin
-    If Not TCrypto.ECDSAVerify(account_signer.accountInfo.accountkey,GetOperationHashToSign(FData),FData.sign) then begin
-      errors := 'Invalid sign';
-      FHasValidSignature := false;
-      exit;
-    end else FHasValidSignature := true;
-    FSignatureChecked:=True;
-  end else begin
-    If Not FHasValidSignature then begin
-      errors := 'Invalid sign';
-      exit;
-    end;
-  end;
+  If Not TCrypto.ECDSAVerify(account_signer.accountInfo.accountkey,GetOperationHashToSign(FData),FData.sign) then begin
+    errors := 'Invalid sign';
+    FHasValidSignature := false;
+    exit;
+  end else FHasValidSignature := true;
 
   FPrevious_Signer_updated_block := account_signer.updated_block;
   FPrevious_Destination_updated_block := account_target.updated_block;
@@ -1873,7 +1838,6 @@ begin
     TLog.NewLog(lterror,Classname,'Error signing a new list account for sale operation');
     FHasValidSignature := false;
   end else FHasValidSignature := true;
-  FSignatureChecked:=True;
 end;
 
 function TOpListAccountForSale.IsDelist: Boolean;
@@ -1901,7 +1865,6 @@ begin
     TLog.NewLog(lterror,Classname,'Error signing a delist account operation');
     FHasValidSignature := false;
   end else FHasValidSignature := true;
-  FSignatureChecked:=True;
 end;
 
 function TOpDelistAccountForSale.IsDelist: Boolean;
@@ -1937,7 +1900,6 @@ begin
     TLog.NewLog(lterror,Classname,'Error signing a new Buy operation');
     FHasValidSignature := false;
   end else FHasValidSignature := true;
-  FSignatureChecked:=True;
 end;
 
 procedure TOpBuyAccount.InitializeData;