Skip to content

Commit

Permalink
fix: update ToRistrettoPoint handling (tari-project#5973)
Browse files Browse the repository at this point in the history
Description
---
Updates the handling of the `ToRistrettoPoint` opcode.

Closes tari-project#5818.

Motivation and Context
---
The `ToRistrettoPoint` opcode now requires that stack input be the
canonical encoding of a Ristretto secret key. This PR updates the opcode
documentation and adds a test for proper handling of invalid encoding.
It also corrects the error returned on invalid stack input.

There is a [separate PR](tari-project/rfcs#113)
that updates the RFC documentation.

How Has This Been Tested?
---
Existing tests pass. A modified test passes.

What process can a PR reviewer use to test or verify this change?
---
Check that the updated documentation reflects the opcode handling. Check
that the modified test correctly detects invalid input. Check that the
error returned on invalid input is correct.
  • Loading branch information
AaronFeickert authored and sdbondi committed Nov 27, 2023
1 parent 3f7e1ad commit adb154b
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
8 changes: 5 additions & 3 deletions infrastructure/tari_script/src/op_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,11 @@ pub enum Opcode {
/// Identical to CheckMultiSig, except that the aggregate of the public keys is pushed to the stack if multiple
/// signature validation succeeds. Fails with `VerifyFailed` if any signature is invalid.
CheckMultiSigVerifyAggregatePubKey(u8, u8, Vec<RistrettoPublicKey>, Box<Message>),
/// Pops the top element from the stack, either a scalar or a hash, calculates the corresponding Ristretto point,
/// and pushes the result to the stack. Fails with `StackUnderflow` if the stack is empty. Fails with
/// `IncompatibleTypes` if the stack item is not a valid 32 byte sequence.
/// Pops the top element from the stack (either a scalar or a hash), parses it canonically as a Ristretto secret
/// key if possible, computes the corresponding Ristretto public key, and pushes this value to the stack.
/// Fails with `StackUnderflow` if the stack is empty.
/// Fails with `IncompatibleTypes` if the stack item is not either a scalar or a hash.
/// Fails with `InvalidInput` if the stack item cannot be canonically parsed as a Ristretto secret key.
ToRistrettoPoint,

// Miscellaneous
Expand Down
17 changes: 12 additions & 5 deletions infrastructure/tari_script/src/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ impl TariScript {
StackItem::Scalar(scalar) => scalar.as_slice(),
_ => return Err(ScriptError::IncompatibleTypes),
};
let ristretto_sk = RistrettoSecretKey::from_canonical_bytes(scalar).map_err(|_| ScriptError::InvalidData)?;
let ristretto_sk = RistrettoSecretKey::from_canonical_bytes(scalar).map_err(|_| ScriptError::InvalidInput)?;
let ristretto_pk = RistrettoPublicKey::from_secret_key(&ristretto_sk);
stack.push(StackItem::PublicKey(ristretto_pk))?;
Ok(())
Expand Down Expand Up @@ -1738,11 +1738,13 @@ mod test {

#[test]
fn to_ristretto_point() {
use crate::StackItem::PublicKey;
use crate::{Opcode::ToRistrettoPoint, StackItem::PublicKey};

// Generate a key pair
let mut rng = rand::thread_rng();
let (k_1, p_1) = RistrettoPublicKey::random_keypair(&mut rng);

use crate::Opcode::ToRistrettoPoint;
// Generate a test script
let ops = vec![ToRistrettoPoint];
let script = TariScript::new(ops);

Expand All @@ -1751,17 +1753,22 @@ mod test {
let err = script.execute(&inputs).unwrap_err();
assert!(matches!(err, ScriptError::IncompatibleTypes));

// scalar
// Valid scalar
let mut scalar = [0u8; 32];
scalar.copy_from_slice(k_1.as_bytes());
let inputs = inputs!(scalar);
let result = script.execute(&inputs).unwrap();
assert_eq!(result, PublicKey(p_1.clone()));

// hash
// Valid hash
let inputs = ExecutionStack::new(vec![Hash(scalar)]);
let result = script.execute(&inputs).unwrap();
assert_eq!(result, PublicKey(p_1));

// Invalid bytes
let invalid = [u8::MAX; 32]; // not a canonical scalar encoding!
let inputs = inputs!(invalid);
assert!(matches!(script.execute(&inputs), Err(ScriptError::InvalidInput)));
}

#[test]
Expand Down

0 comments on commit adb154b

Please sign in to comment.