-
Notifications
You must be signed in to change notification settings - Fork 116
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
Upgrade to AIP-62 standard #464
base: main
Are you sure you want to change the base?
Conversation
b81b5f7
to
a79eab7
Compare
b7898ef
to
042eb70
Compare
).transaction.build.simple({ | ||
sender: account.address, | ||
data: { | ||
function: "0x1::coin::transfer", | ||
typeArguments: [APTOS_COIN], | ||
functionArguments: [account.address, 1], // 1 is in Octas | ||
functionArguments: [account.address.toString(), 1], // 1 is in Octas |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not completely sure why it fails to send the account.address
instance here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mm good question. maybe AccountAddress
from a different js bundle?
so instanceof AccountAddress
returns false internally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes maybe, I thought that also. But both example and core use the same version, I'll test it again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested, and made sure both packages use the same sdk version, still fails. It does not seem to be a sdk issue also. Everything is weird
@@ -935,135 +714,100 @@ export class WalletCore extends EventEmitter<WalletCoreEvents> { | |||
* @returns AccountAuthenticator | |||
*/ | |||
async signTransaction( | |||
transactionOrPayload: AnyRawTransaction | Types.TransactionPayload, | |||
transactionOrPayload: AnyRawTransaction | InputTransactionData, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe split to 2 functions, one for JSON and one for Raw transaction input
ee4cd0c
to
d08f4e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks overall good! So much cleaner :D
// { | ||
// label: "Min keys required", | ||
// subLabel: "(only for multisig)", | ||
// value: ( | ||
// <p>{account?.minKeysRequired?.toString() ?? "Not Present"}</p> | ||
// ), | ||
// }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this not working at the moment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the standard does not consider multi sig
).transaction.build.simple({ | ||
sender: account.address, | ||
data: { | ||
function: "0x1::coin::transfer", | ||
typeArguments: [APTOS_COIN], | ||
functionArguments: [account.address, 1], // 1 is in Octas | ||
functionArguments: [account.address.toString(), 1], // 1 is in Octas |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mm good question. maybe AccountAddress
from a different js bundle?
so instanceof AccountAddress
returns false internally
3563cc9
to
53eee94
Compare
53eee94
to
ad886d2
Compare
This PR migrates the wallet adapter to support only the AIP-62 format and compatible wallets.
The AIP-62 standard is a chain-agnostic set of interfaces and conventions designed to enhance how applications interact with injected wallets. See more https://github.com/aptos-foundation/AIPs/blob/main/aips/aip-62.md
By migrating to AIP-62, we can deprecate unmaintained or unused Aptos wallets, ensuring support is provided only for actively developed and functional wallets.
Additionally, dApps will be able to fetch and utilize wallet type information—a feature that has been highly requested by both the community and internal developers, see https://aptos-org.slack.com/archives/C06TWPH09Q9/p1721771598556349)
Live demo https://aptos-labs.github.io/aptos-wallet-adapter/nextjs-example-testing/