Skip to content

Commit

Permalink
Modify transaction struct itself when calling sign() on it
Browse files Browse the repository at this point in the history
Instead of returning a signed copy. I think this API is more intuitive.

I also don't want to do both, modify self and return a signed copy (because it's not possible to just return self, that is first-of-all a compiler error, and secondly would create a second instance in jS-land anyway), as that would transparently create a copy of the variable, which is unexpected behavior for users of the library and will lead only to errors.
  • Loading branch information
sisou committed Aug 10, 2023
1 parent 186d976 commit 01ff093
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 8 deletions.
15 changes: 10 additions & 5 deletions web-client/example/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ init().then(async () => {
BigInt(fee),
await client.getHeadHeight(),
await client.getNetworkId(),
).sign(keyPair);
);
transaction.sign(keyPair);
} else {
transaction = Nimiq.TransactionBuilder.newBasic(
keyPair.toAddress(),
Expand All @@ -77,7 +78,8 @@ init().then(async () => {
BigInt(fee),
await client.getHeadHeight(),
await client.getNetworkId(),
).sign(keyPair);
);
transaction.sign(keyPair);
}

return client.sendTransaction(transaction.toHex());
Expand All @@ -104,7 +106,8 @@ init().then(async () => {
BigInt(fee),
await client.getHeadHeight(),
await client.getNetworkId(),
).sign(keyPair);
);
transaction.sign(keyPair);

return client.sendTransaction(transaction.toHex());
}
Expand All @@ -128,7 +131,8 @@ init().then(async () => {
BigInt(fee),
await client.getHeadHeight(),
await client.getNetworkId(),
).sign(keyPair);
);
transaction.sign(keyPair);

return client.sendTransaction(transaction.toHex());
}
Expand All @@ -152,7 +156,8 @@ init().then(async () => {
BigInt(fee),
await client.getHeadHeight(),
await client.getNetworkId(),
).sign(keyPair);
);
transaction.sign(keyPair);

return client.sendTransaction(transaction.toHex());
}
Expand Down
7 changes: 4 additions & 3 deletions web-client/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,11 @@ impl Transaction {
///
/// ### Limitations
/// - HTLC redemption is not supported and will throw.
/// - Validator deletion transactions are not and cannot be supported.
/// - For transaction to the staking contract, both signatures are made with the same keypair,
/// so it is not possible to interact with a staker that is different from the sender address
/// or using a different cold or signing key for validator transactions.
#[cfg(feature = "primitives")]
pub fn sign(&self, key_pair: &KeyPair) -> Result<Transaction, JsError> {
pub fn sign(&mut self, key_pair: &KeyPair) -> Result<(), JsError> {
let proof_builder = TransactionProofBuilder::new(self.native_ref().clone());
let signed_transaction = match proof_builder {
TransactionProofBuilder::Basic(mut builder) => {
Expand Down Expand Up @@ -196,7 +195,9 @@ impl Transaction {
}
};

Ok(Transaction::from_native(signed_transaction))
self.set_proof(signed_transaction.proof);

Ok(())
}

/// Computes the transaction's hash, which is used as its unique identifier on the blockchain.
Expand Down

0 comments on commit 01ff093

Please sign in to comment.