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

Fix invalid account data for instruction error #1241

Merged
merged 4 commits into from
Nov 8, 2024
Merged

Conversation

aminsato
Copy link
Contributor

@aminsato aminsato commented Oct 25, 2024

The objective of this PR is to address the issue that arises when a user transfers a Solana token to an account that lacks the token, resulting in an Instruction data not found error.

In the event that the recipient Solana account does not possess the token, this PR system will automatically generate a token account for the recipient Solana account.

@aminsato aminsato requested review from yvebe and johnnyluo and removed request for yvebe October 25, 2024 03:01
@aminsato aminsato requested a review from enriquesouza October 25, 2024 03:07
@johnnyluo
Copy link
Contributor

Please attach link to the transaction that you sent here

@@ -40,6 +40,7 @@ sealed class BlockChainSpecific {
val priorityFee: BigInteger,
val fromAddressPubKey: String? = null,
val toAddressPubKey: String? = null,
val tokenAccountExists: Boolean? = null,
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not work , as this change is not in the protobuf , thus it doesn't get passed to pair device
and this should caues keysign failure , because they will not agree on what they are signing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I updated the "common data" submodule.

vultisig/commondata#16
Screenshot_20241025_092434_Chrome

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need to add new stuff to commondata now , please refer to how IOS handle it
https://github.com/vultisig/vultisig-ios/blob/ton-stake-function/VultisigApp/VultisigApp/Chains/Solana.swift#L44

Copy link
Contributor Author

@aminsato aminsato Oct 25, 2024

Choose a reason for hiding this comment

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

There is no need to add new stuff to commondata now , please refer to how IOS handle it https://github.com/vultisig/vultisig-ios/blob/ton-stake-function/VultisigApp/VultisigApp/Chains/Solana.swift#L44

we don't actually make a Solana account. What we do is create a token account address for the Solana account we want to use, if it doesn't already have one..

Copy link
Contributor Author

@aminsato aminsato Oct 25, 2024

Choose a reason for hiding this comment

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

This will not work , as this change is not in the protobuf , thus it doesn't get passed to pair device and this should caues keysign failure , because they will not agree on what they are signing

The Android code currently aligns with the iOS code.

if (solanaSpecific.fromAddressPubKey != null && solanaSpecific.toAddressPubKey!= null) {
val transfer = Solana.TokenTransfer.newBuilder()
.setTokenMintAddress(keysignPayload.coin.contractAddress)
.setSenderTokenAddress(solanaSpecific.fromAddressPubKey)
.setRecipientTokenAddress(solanaSpecific.toAddressPubKey)
.setAmount(keysignPayload.toAmount.toLong())
.setDecimals(keysignPayload.coin.decimal)

https://github.com/vultisig/vultisig-ios/blob/77da3dbec92dfbe77fc36242cbf7cf0b1f7fb151/VultisigApp/VultisigApp/Chains/Solana.swift#L46-L54

Copy link
Contributor Author

@aminsato aminsato Oct 25, 2024

Choose a reason for hiding this comment

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

The current iOS version does not address the issue.

@johnnyluo
Copy link
Contributor

@enriquesouza did we address the same issue on IOS?

@aminsato
Copy link
Contributor Author

Please attach link to the transaction that you sent here

#1205 (comment)

@aminsato aminsato changed the title Fix Instruction data not found error Fix invalid account data for instruction error Oct 25, 2024
@enriquesouza
Copy link

@aminsato aminsato requested a review from johnnyluo November 2, 2024 17:22
@aminsato
Copy link
Contributor Author

aminsato commented Nov 3, 2024

D5GdnAGgE1YmmWHcEgrVbC3kHTCuszWfyVgvsfrQHZA3

@aminsato aminsato self-assigned this Nov 5, 2024
@johnnyluo
Copy link
Contributor

@yvebe This one LGTM , should we get it in?

@yvebe yvebe merged commit e13bf6e into main Nov 8, 2024
1 check passed
@yvebe yvebe deleted the fix/send-spl-error branch November 8, 2024 11:30
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.

4 participants