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

Minor enhancements #135

Merged
merged 12 commits into from
Sep 20, 2022
Merged

Minor enhancements #135

merged 12 commits into from
Sep 20, 2022

Conversation

davidmisiak
Copy link
Collaborator

This PR fixes #120, #126, #131, #132, #134 and allows non-Plutus signing modes for txs with required signers. Most of the changes are rather small and simple, therefore only a single PR instead of six, but I recommend reviewing commit-by-commit.

@gitmachtl
Copy link
Contributor

gitmachtl commented Sep 15, 2022

#132 so the restriction has been lifted and now the payment key from a hw-wallet can also be used for any kind of pool-registration/pool-update transactions?

#131 😃

@davidmisiak
Copy link
Collaborator Author

#132 so the restriction has been lifted and now the payment key from a hw-wallet can also be used for any kind of pool-registration/pool-update transactions?

No, pool registration still cannot be signed with a payment key as a pool owner (i.e. by a pyament and a staking key at the same time). Signing only with a payment key as the pool operator was allowed by the Ledger app (if I'm not mistaken), it just hasn't been supported by hw-cli.

Copy link
Collaborator

@janmazak janmazak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a little refactor of isByronPath, but LGTM. I've not run any tests, nor searched all the source code for missed items to rename etc.

test/integration/ledger/node/tx.js Show resolved Hide resolved
src/types.ts Show resolved Hide resolved
src/crypto-providers/util.ts Outdated Show resolved Hide resolved
@gitmachtl
Copy link
Contributor

For release notes 1.12.0:

  • keeping original auxiliary data hash on transaction transform when the tx doesn't contain auxiliary data (cardano-hw-interop-lib)

that line is a bit misleading. the auxdata hash is kept if no auxiliary data is in the tx, and its corrected if the auxdata was reordered in canonical order. or is the auxdata not reordered anymore?

@davidmisiak
Copy link
Collaborator Author

the auxdata hash is kept if no auxiliary data is in the tx, and its corrected if the auxdata was reordered in canonical order

Yes, that's the case. I can try to clarify that in the release notes.

@janmazak janmazak deleted the minor-enhancements branch February 6, 2023 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor passing input paths to Ledgerjs and Connect
3 participants