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

[remotesigning] Refactor destination validator to only use master seed #143

Merged
merged 4 commits into from
Dec 13, 2024

Conversation

alecchendev
Copy link
Contributor

@alecchendev alecchendev commented Dec 12, 2024

Currently, for all signing jobs, we derive Xpubs from the signing derivation path, and pass them into the validation interface. There's a couple issues with this:

  1. These are using the wrong derivation paths. We pass Xpubs derived from the prefix of the derivation path into the validator so it can check if the destination of a transaction is derived from our seed. We should be using the destination derivation path, not the signing derivation path. This misleadingly works for L1 wallet signing requests, because the signing derivation path is the same as the destination (the prefix at least, for withdrawals).
  2. We end up doing this weird thing where we derive part of the public key, then the other later, which just adds extra code/logic.
  3. These Xpubs are not used at all for non-L1-wallet requests, and they aren't used for any validation by the HashValidator.

This PR refactors things so we simply provide the master seed to the validator, then derive pubkeys from there and pass it into the ValidateScript function.

@alecchendev alecchendev requested a review from zhenlu December 12, 2024 23:52
Copy link
Contributor

@zhenlu zhenlu left a comment

Choose a reason for hiding this comment

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

oops

Error: ./server.go:40:15: cannot use remotesigning.PositiveValidator{} (value of type remotesigning.PositiveValidator) as remotesigning.Validator value in assignment: remotesigning.PositiveValidator does not implement remotesigning.Validator (wrong type for method ShouldSign)

@alecchendev alecchendev force-pushed the 12-alec-validate-interface branch from 56d91cc to ca84c78 Compare December 12, 2024 23:55
@alecchendev
Copy link
Contributor Author

fixed!

@alecchendev alecchendev requested a review from zhenlu December 13, 2024 02:59
@alecchendev alecchendev merged commit c366102 into main Dec 13, 2024
2 checks passed
@alecchendev alecchendev deleted the 12-alec-validate-interface branch December 13, 2024 05:29
alecchendev added a commit that referenced this pull request Dec 16, 2024
In #143 this function became unused.
alecchendev added a commit that referenced this pull request Dec 16, 2024
In #143 this function became unused.
alecchendev added a commit that referenced this pull request Dec 16, 2024
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.

2 participants