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

Combine scripts generation within transaction creation methods #11

Closed
jrwbabylonlab opened this issue Aug 19, 2024 · 2 comments
Closed
Assignees

Comments

@jrwbabylonlab
Copy link
Collaborator

jrwbabylonlab commented Aug 19, 2024

Right now the lib very much like a functional programming where creating/signing a staking tx require u call a method to create scripts, then feed into another method for signing etc.
This has its own downside as the important variables are being passed around which is less secure and prone to bugs with limited validations in place.

The scope of this issue is to have a think about the above problem and assess whether we need a new design for this lib

Recommendation:

  • Leave transaction and scripts creation implementation as it is, but create a new set of methods/class where it will combine the scripts and transaction together under a single method. For example, the createStakingTx from simple-staking to be abstracted and moved into this lib instead.
jrwbabylonlab pushed a commit that referenced this issue Aug 21, 2024
@jrwbabylonlab jrwbabylonlab changed the title Redesign the lib data structure Combine scripts generation within transaction creation methods Aug 22, 2024
@jrwbabylonlab
Copy link
Collaborator Author

jrwbabylonlab commented Aug 22, 2024

Once the improvement is done, it would also resolve below issue out of the box:

Seems like (publicKeyNoCoord)[https://github.com/babylonlabs-io/btc-staking-ts/blob/dev/src/index.ts#L67] is the stakerkey
when using taproot mode of the wallet. There is no check that this key and key in provided scripts matches.

and

There is no check that the op_return (https://github.com/babylonlabs-io/btc-staking-ts/blob/dev/src/index.ts#L60),
is in fact op_return recognised by Babylon and contains the same data that in included in scripts.

@jrwbabylonlab
Copy link
Collaborator Author

also linking another issue which i believe will be addressed as part of this refactoring #9

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

No branches or pull requests

1 participant