Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
- Simplify SignTransaction result creation (more in line with how it was before the switch to PoS)
- Explain swap refund transaction validity-start-height calculation drawbacks and lower-bound it to the HTLC creation VSH
  • Loading branch information
sisou committed Jan 14, 2025
1 parent cb2f92c commit f9c5d36
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 12 deletions.
15 changes: 5 additions & 10 deletions src/request/sign-transaction/SignTransaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,21 +182,16 @@ class SignTransaction {
return;
}

const privateKey = key.derivePrivateKey(request.keyPath);
const publicKey = Nimiq.PublicKey.derive(privateKey);
const publicKey = key.derivePublicKey(request.keyPath);
const signature = key.sign(request.keyPath, request.transaction.serializeContent());

const tx = request.transaction;

// Manually create signature proof, because tx.sign(keyPair) does not support HTLC redeeming transactions.
const signature = Nimiq.Signature.create(privateKey, publicKey, tx.serializeContent());
const proof = Nimiq.SignatureProof.singleSig(publicKey, signature);
tx.proof = proof.serialize();
request.transaction.proof = Nimiq.SignatureProof.singleSig(publicKey, signature).serialize();

/** @type {KeyguardRequest.SignTransactionResult} */
const result = {
publicKey: publicKey.serialize(),
signature: tx.proof.subarray(tx.proof.length - 64),
serializedTx: tx.serialize(),
signature: signature.serialize(),
serializedTx: request.transaction.serialize(),
};
resolve(result);
}
Expand Down
24 changes: 22 additions & 2 deletions src/request/swap-iframe/SwapIFrameApi.js
Original file line number Diff line number Diff line change
Expand Up @@ -429,14 +429,34 @@ class SwapIFrameApi extends BitcoinRequestParserMixin(RequestParser) { // eslint
const feePerUnit = Number(transaction.fee) / transaction.serializedSize;
const fee = BigInt(Math.ceil(feePerUnit * 167)); // 167 = NIM HTLC refunding tx size

/**
* Calculate future ValidityStartHeight for the refund transaction.
*
* This is not exact, unfortunately. But hopefully close enough. The calculated VSH is always
* smaller than it should be (blocks are faster than 1/second on average, time taken between
* request creation and this code is subtracted as well). Additionally, if the user's system
* clock is off, the resulting VHS will be wrong, too.
*
* But the watchtower only sends the refund transaction once the timeout _timestamp_ of the swap
* has passed, at which point the transaction will still be valid (except if totally wrong because
* of system clock).
*
* If the refund transaction is invalid when it is supposed to be sent, the user can always
* trigger a refund manually from the transaction details in the Wallet.
*/
const validityStartHeight = Math.max(
transaction.validityStartHeight,
transaction.validityStartHeight
+ (parsedRequest.fund.htlcDetails.timeoutTimestamp - Math.round(Date.now() / 1e3)),
);

// Create refund transaction
const refundTransaction = new Nimiq.Transaction(
transaction.recipient, Nimiq.AccountType.HTLC, new Uint8Array(0),
transaction.sender, Nimiq.AccountType.Basic, new Uint8Array(0),
transaction.value - fee, fee,
Nimiq.TransactionFlag.None,
transaction.validityStartHeight
+ (parsedRequest.fund.htlcDetails.timeoutTimestamp - Math.round(Date.now() / 1e3)),
validityStartHeight,
CONFIG.NIMIQ_NETWORK_ID,
);

Expand Down

0 comments on commit f9c5d36

Please sign in to comment.