soroban-cli icon indicating copy to clipboard operation
soroban-cli copied to clipboard

feat: add tx sign/send and Stellar trait

Open willemneal opened this issue 1 year ago • 1 comments

What

~Currently relies on #1283~

Adds:

  • tx sign - sign just a transaction (envelope), appending the signature
    • Does not have --source-acount and requires --sign-with
    • In a future PR tx auth-sign/auth will be added to handle signing just authorization entries.
  • tx send - submit a signed transaction envelope to the network
  • Stellar trait which handles the signing of transactions and auth-entries
  • Updates signing across all commands.
  • For invoke command (currently only command that handles signing auth entries), but these will used with tx auth:
    • --sign-with-key - takes same input as --source-account
    • --sign-with-lab - currently does nothing
    • --yes - if --sign-with-key user verification is required, unless --yes is provided

Why

Currently signing happens automatically in the background, which is unsure and doesn't inform users when signing has taken place.

Known limitations

Doesn't add ledger support. Still need to figure out dep issues. This is to be done in follow up PR.

willemneal avatar Jun 25 '24 16:06 willemneal

The refactor looks fine, although we don't have much in the way of testing which we should address.

@leighmcculloch I relied on previous tests since this logic touched so many commands. Do you think we need more to approve the PR?

willemneal avatar Jul 30 '24 18:07 willemneal

@Ifropc @leighmcculloch

I wanted to present some thoughts for how this all could be refactored. I have tested it locally.

It's a rather big change, but I propose breaking everything into smaller traits and then make use of blank implementations. I can make a follow up issue, but wanted to drop it here for thoughts and to see if it's good enough for merging this PR as is. I'm also not married to this idea, but think it is more inline with the smaller traits route. We could also just not worry with blanket impls and just implement them for each type since there weren't be a ton of signers, just thought it could help make it easier to write them.


/// A trait for signing Stellar transaction Envelopes
#[async_trait::async_trait]
pub trait TxEnv {
    /// Sign a Stellar  with the given source account
    /// This is a default implementation that signs the transaction hash and returns a decorated signature
    ///
    /// Todo: support signing the transaction directly.
    /// # Errors
    /// Returns an error if the source account is not found
    async fn sign_txn_env(
        &self,
        txn_env: TransactionEnvelope,
        network: &Network,
    ) -> Result<TransactionEnvelope, Error>;
    // let hash = transaction_hash(txn, network_passphrase)?;
    // self.sign_txn_hash(hash).await
}

/// A trait for signing Stellar transactions
#[async_trait::async_trait]
pub trait Tx {
    /// Sign a Stellar transaction with the given source account
    /// This is a default implementation that signs the transaction hash and returns a decorated signature
    ///
    /// Todo: support signing the transaction directly.
    /// # Errors
    /// Returns an error if the source account is not found
    async fn sign_txn(
        &self,
        txn: &Transaction,
        network: &Network,
    ) -> Result<DecoratedSignature, Error>;
}

/// A trait for signing Stellar transaction Hashes
#[async_trait::async_trait]
pub trait TxHash {
    /// Sign a transaction hash with the given source account
    /// # Errors
    /// Returns an error if the source account is not found
    async fn sign_txn_hash(&self, txn: [u8; 32]) -> Result<DecoratedSignature, Error>;
}

#[async_trait::async_trait]
impl<T: Blob + HasPublicKey + std::marker::Sync> TxHash for T {
    async fn sign_txn_hash(&self, txn: [u8; 32]) -> Result<DecoratedSignature, Error> {
        let source_account = self.get_public_key().await?;
        eprintln!(
            "{} about to sign hash: {}",
            source_account.to_string(),
            hex::encode(txn)
        );
        let tx_signature = self.sign_blob(&txn).await?;
        Ok(DecoratedSignature {
            // TODO: remove this unwrap. It's safe because we know the length of the array
            hint: SignatureHint(source_account.0[28..].try_into().unwrap()),
            signature: Signature(tx_signature.try_into()?),
        })
    }
}

#[async_trait::async_trait]
impl<T: TxHash + std::marker::Sync> Tx for T {
    async fn sign_txn(
        &self,
        txn: &Transaction,
        Network {
            network_passphrase, ..
        }: &Network,
    ) -> Result<DecoratedSignature, Error> {
        self.sign_txn_hash(transaction_hash(txn, network_passphrase)?)
            .await
    }
}

/// A trait for signing abritrary byte arrays
#[async_trait::async_trait]

pub trait Blob {
    /// Sign an abritatry byte array
    async fn sign_blob(&self, blob: &[u8]) -> Result<Vec<u8>, Error>;
}

#[async_trait::async_trait]
pub trait HasPublicKey {
    /// Get the public key of the signer
    async fn get_public_key(&self) -> Result<stellar_strkey::ed25519::PublicKey, Error>;
}

#[async_trait::async_trait]
impl<T: Tx + std::marker::Sync> TxEnv for T {
    async fn sign_txn_env(
        &self,
        txn_env: TransactionEnvelope,
        network: &Network,
    ) -> Result<TransactionEnvelope, Error> {
        match txn_env {
            TransactionEnvelope::Tx(TransactionV1Envelope { tx, signatures }) => {
                let decorated_signature = self.sign_txn(&tx, network).await?;
                let mut sigs = signatures.to_vec();
                sigs.push(decorated_signature);
                Ok(TransactionEnvelope::Tx(TransactionV1Envelope {
                    tx,
                    signatures: sigs.try_into()?,
                }))
            }
            _ => Err(Error::UnsupportedTransactionEnvelopeType),
        }
    }
}

pub(crate) fn hash(network_passphrase: &str) -> xdr::Hash {
    xdr::Hash(Sha256::digest(network_passphrase.as_bytes()).into())
}

pub struct LocalKey {
    key: ed25519_dalek::SigningKey,
    prompt: bool,
}

impl LocalKey {
    pub fn new(key: ed25519_dalek::SigningKey, prompt: bool) -> Self {
        Self { key, prompt }
    }
}

#[async_trait::async_trait]
impl Blob for LocalKey {
    async fn sign_blob(&self, data: &[u8]) -> Result<Vec<u8>, Error> {
        if self.prompt {
            eprintln!("Press 'y' or 'Y' for yes, any other key for no:");
            match read_key() {
                'y' | 'Y' => {
                    eprintln!("Signing now...");
                }
                _ => return Err(Error::UserCancelledSigning),
            };
        }
        let sig = self.key.sign(data);
        Ok(sig.to_bytes().to_vec())
    }
}
#[async_trait::async_trait]
impl HasPublicKey for LocalKey {
    async fn get_public_key(&self) -> Result<stellar_strkey::ed25519::PublicKey, Error> {
        Ok(stellar_strkey::ed25519::PublicKey(
            self.key.verifying_key().to_bytes(),
        ))
    }
}

willemneal avatar Aug 15 '24 18:08 willemneal

Hey @willemneal thanks for detailed suggestion, I like the approach of breaking it down into smaller pieces!

ifropc avatar Aug 16 '24 23:08 ifropc

Thanks @Ifropc for reviewing it. @fnando and I are also reviewing.

leighmcculloch avatar Aug 27 '24 03:08 leighmcculloch

closed in favor of: https://github.com/stellar/stellar-cli/issues/1591

willemneal avatar Sep 10 '24 21:09 willemneal