algorand / algorand/js-algorand-sdk
signTransaction should refuse to sign any txn with lease not exactly 32 bytes
- Dominant language
- TypeScript
- Stars
- 297
- Forks
- 214
- Avg merge
- 1h 3m
- Merged PRs (30d)
- 3
Description
### signTransaction should refuse to sign any txn with lease not exactly 32 bytes
Currently if you manually set a `.lease` field on a transaction that is not 32 bytes, algosdk will happily sign it, and when posting the transaction to algod you get `At least one signature didn't pass verification`
If a user needs to use the lease feature with non-app-call txn, they must add it manually, as most makeTxn methods (eg `makePaymentTxnWithSuggestedParamsFromObject`) do not support `.lease` - which could also be remedied in the future.
If was [recommended on the forum](https://forum.algorand.org/t/brainstorm-on-default-valid-window-length-and-lease/8208/3) that I open an issue about this.
Repro code:
```
const pay = algosdk.makePaymentTxnWithSuggestedParamsFromObject({
from: creator,
to: creator,
amount: 0,
// lease here is ignored, could be improved to support it
suggestedParams: params,
});
// invalid lease - must be exactly 32 bytes
pay.lease = new Uint8Array(Buffer.from('not 32 byte=weird fail'));
// this works but shouldn't
const signedTx = algosdk.signTransaction(pay, recoveredAccount.sk);
// this fails with "At least one signature didn't pass verification"
let sendTx = await algodClient.sendRawTransaction(signedTx.blob).do();
```
The same code with a 32-byte lease value works as expected
### Your environment
algosdk: 1.23.2
OS: Ubuntu 22.04
### Steps to reproduce
1. Use code like the provided snippet to sign & send a transaction with `.lease` that is set but not exactly 32 bytes in length
### Expected behaviour
signTransaction refuses to sign a txn with an invalid lease field
### Actual behaviour
signTransaction signs, and algod fails with a generic error message: `At least one signature didn't pass verification`
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.