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

[Discussion] Type proxies in btc: Address and Pubkey #235

Open
ShookLyngs opened this issue Jun 18, 2024 · 2 comments
Open

[Discussion] Type proxies in btc: Address and Pubkey #235

ShookLyngs opened this issue Jun 18, 2024 · 2 comments

Comments

@ShookLyngs
Copy link
Collaborator

ShookLyngs commented Jun 18, 2024

We can define type proxies for Address and Pubkey to improve the code readability of the btc lib. However, it would take some time to search for relevant code and update it. So, the discussion is: Do you think we should define the two type proxies, and do you think the change is worthwhile?

From this:

interface SendXProps {
  from: string;
  fromPubkey?: string;
  pubkeyMap?: Record<string, string>; // Record<address, pubkey>
}

Which can be refactored to this:

type Address = string;
type Pubkey = string;

interface SendXProps {
  from: Address;
  fromPubkey?: Pubkey;
  pubkeyMap?: Record<Address, Pubkey>;
}

Original commented at: #228 (comment)

The original comment was discussing about how to make the type of pubkeyMap more clear, in the PR I've chosen a simpler solution to only define a type AddressToPubkeyMap with an explanation comment:

/**
 * Type: Record<Address, Pubkey>
 * The map of address and pubkey, usually for recognizing the P2TR inputs in the transaction.
 */
type AddressToPubkeyMap = Record<string, string>;

interface SendXProps {
  from: string;
  fromPubkey?: string;
  pubkeyMap?: AddressToPubkeyMap;
}
@duanyytop
Copy link
Collaborator

duanyytop commented Jun 18, 2024

I think refactoring is the icing on the cake. The amount of work required and whether it is worth spending time depends on your evaluation.

If you think the input-output ratio is low, we can do nothing for it.

I want to emphasize that this is just a suggestion.

@Flouse
Copy link
Contributor

Flouse commented Jun 18, 2024

in the PR I've chosen a simpler solution to only define a type AddressToPubkeyMap with an explanation comment:

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

3 participants