Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing undefined check #1768

Open
aryzing opened this issue Nov 21, 2024 · 2 comments
Open

Add missing undefined check #1768

aryzing opened this issue Nov 21, 2024 · 2 comments
Assignees
Labels
bug Unwanted or unintended logic causing harm

Comments

@aryzing
Copy link

aryzing commented Nov 21, 2024

What version of Stacks.js are you using?

7.0.3

Describe the bug

Seems the makeUnsignedSTXTokenTransfer builder is missing an undefined check for automatically setting the fee and nonce,

if (txOptions.fee == null) {
const fee = await fetchFeeEstimate({ transaction, ...options });
transaction.setFee(fee);
}
if (txOptions.nonce == null) {
const addressVersion = options.network.addressVersion.singleSig;
const address = c32address(addressVersion, transaction.auth.spendingCondition!.signer);
const txNonce = await fetchNonce({ address, ...options });
transaction.setNonce(txNonce);
}

in contrast to,

if (txOptions.fee === undefined || txOptions.fee === null) {
const fee = await fetchFeeEstimate({ transaction, ...options });
transaction.setFee(fee);
}
if (txOptions.nonce === undefined || txOptions.nonce === null) {
const addressVersion = options.network.addressVersion.singleSig;
const address = c32address(addressVersion, transaction.auth.spendingCondition!.signer);
const txNonce = await fetchNonce({ address, ...options });
transaction.setNonce(txNonce);
}

Expected behavior

All builders should have both null + undef check to when auto-setting fee & nonce

@aryzing aryzing added the bug Unwanted or unintended logic causing harm label Nov 21, 2024
@github-project-automation github-project-automation bot moved this to 🆕 New in DevTools Nov 21, 2024
@aryzing
Copy link
Author

aryzing commented Nov 21, 2024

My bad, didn't realize it was using the == operator instead of ===, which does match undefined to null. Maybe worth using something more explicit?

  • like Leather's isUndefined
  • or something like isNullish (inspired by nullish) to check for either undefined or null?

@janniks
Copy link
Collaborator

janniks commented Dec 16, 2024

Yes thanks for noting there's a few less explicit uses like this. Will add to the refactoring roadmap

@janniks janniks moved this from 🆕 New to 📋 Backlog in DevTools Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unwanted or unintended logic causing harm
Projects
Status: 📋 Backlog
Development

No branches or pull requests

2 participants