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 docs for MultiChain methods #30

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Add docs for MultiChain methods #30

wants to merge 12 commits into from

Conversation

Pessina
Copy link
Contributor

@Pessina Pessina commented Jun 13, 2024

No description provided.

benmalcom
benmalcom previously approved these changes Jun 13, 2024
Copy link
Contributor

@benmalcom benmalcom left a comment

Choose a reason for hiding this comment

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

@Pessina this looks good and understandable!

We can also (in the future) export a type intellisense for the fastAuthWallet instance.

chainId: 11155111n,
};

await fastAuthWallet.signAndSendMultiChainTransaction(evmTransactionData);
Copy link
Collaborator

Choose a reason for hiding this comment

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

it's currently called signMultiChainTransaction even though I prefer this naming. I think it's still early enough to change it without a major release? We still have to coordinate with near-multichain-demo though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, as it isn't announced as mainnet, I believe we can introduce breaking changes without a major release. What do you think?

Regarding sync with near-multichain-demo it's already synced on this PR: near/near-multichain-demo#12

@Pessina
Copy link
Contributor Author

Pessina commented Jun 18, 2024

@Pessina this looks good and understandable!

We can also (in the future) export a type intellisense for the fastAuthWallet instance.

Cool, thanks @benmalcom for the review.

Regarding the Intellisense, it comes from the interface defined on the wallet-selector as you can check hovering this line: https://github.com/near/fast-auth-signer/blob/9f65f53c0537ac352a9871495d7b6c949a74eee7/packages/near-fast-auth-signer-e2e-tests/test-app/App.tsx#L16

Screenshot 2024-06-18 at 16 37 08

I believe we need a NEP to change this interface.

Please correct if I misunderstood your suggestion, or don't see the solution you proposed.

I think what you proposed it's a great improvement, but it will come in the future.

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.

3 participants