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

Support for the Finalized W5 (V5R1) Contract #210

Merged
merged 5 commits into from
Jul 18, 2024

Conversation

Totemancer
Copy link
Contributor

@Totemancer Totemancer commented Jul 2, 2024

(WIP - Do not merge yet)

Hello tonutils community and @xssnick!

We have refactored V5R1 to V5Beta. The Ton Core recommends calling it beta to prevent the premature spread of the Beta contract. Additionally, we have prepared support for the V5 Final contract (latest revision).

We can now wait for finalized contracts and update to the new naming and contract once it’s ready to go.

I’m submitting this pull request here so the community can test their software before the big day.

Have fun, and feel free to comment or make adjustments.

from upstream tonkeeper/w5 compiled
base64 IINLe3KxEhR+Gy+0V7hOdNGjDwT3N9T2KmaOlVLSty8=
@Totemancer
Copy link
Contributor Author

The V5 repo was just updated with the finalized contract.
https://github.com/tonkeeper/w5

Happy testing, everyone! Now this can be merged to the dev branch and adjusted further to be ready for the official wallet V5 release.


Eventually, we can implement similar message.ActionType to the struct for full V5 implementation. Currently, it only supports send_msg [0x0ec3c86d].

Similarly, the v5_final.go (packV5FinalActions) can be updated in the near future, but that requires a more in-depth rewrite of the codebase.

                case "add_ext":
			msg = cell.BeginCell().MustStoreUInt(0x02, 32). // action_add_ext prefix
				MustStoreRef(outMsg)                        // message reference
		case "delete_ext":
			msg = cell.BeginCell().MustStoreUInt(0x03, 32). // action_delete_ext prefix
				MustStoreRef(outMsg)                        // message reference
		case "set_signature_auth_allowed":
			msg = cell.BeginCell().MustStoreUInt(0x04, 32). // action_set_signature_auth_allowed prefix
				MustStoreUInt(uint64(message.Mode), 1)       // allowed

@xssnick
Copy link
Owner

xssnick commented Jul 4, 2024

Hi, thank you! But let’s leave the names for backward compatibility and flexibility, Beta = V5R1, Final = V5R2, there is no guarantee that in future it will be a new revision :)

But we can add a comments for constants where mention what is final and what is beta, so it will be easier to understand.

ton/wallet/v5_final.go Outdated Show resolved Hide resolved
@@ -84,8 +88,8 @@ func GetStateInit(pubKey ed25519.PublicKey, version VersionConfig, subWallet uin
MustStoreSlice(pubKey, 256).
MustStoreDict(nil). // empty dict of plugins
EndCell()
case V5R1:
config := version.(ConfigV5R1)
case V5Beta:
Copy link
Owner

Choose a reason for hiding this comment

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

Lets keep name V5R1, and instead of Final call it V5R2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The official name will be V5R1, and the previous W5 is V5 Beta. It is worth keeping V5R1 to avoid confusing new users of the library and to communicate that the previous version of V5 will be V5 Beta. A smaller number of users used the Beta contract, who will use the library in the future.

@Totemancer Totemancer changed the title Refactor V5R1 to V5Beta and Prepare Support for V5 Final (WIP) Support for V5R2 Finalized W5 Contract Jul 4, 2024
@Totemancer Totemancer changed the title Support for V5R2 Finalized W5 Contract Support for the Finalized W5 (V5R2) Contract Jul 4, 2024
@Totemancer
Copy link
Contributor Author

Hi, thank you! But let’s leave the names for backward compatibility and flexibility, Beta = V5R1, Final = V5R2, there is no guarantee that in future it will be a new revision :)

But we can add a comments for constants where mention what is final and what is beta, so it will be easier to understand.

Made the requested changes.

The only thing left is to wait for Tonkeeper and other wallets to integrate it. Hopefully, we will generate the same wallets as the other wallets. In theory, this should be due to the network ID and subwallet IDs matching.

https://github.com/Totemancer/tonutils-go/blob/v5final/ton/wallet/address.go#L117

Let us know if you need any other updates.

@Totemancer Totemancer requested a review from xssnick July 4, 2024 14:43
@Totemancer
Copy link
Contributor Author

Seems will be called V5R1 and previous one is V5Beta

tonkeeper/tonkeeper-ton@c4ca401

@Totemancer Totemancer changed the title Support for the Finalized W5 (V5R2) Contract Support for the Finalized W5 (V5R1) Contract Jul 5, 2024
@Totemancer
Copy link
Contributor Author

Using the official naming and reviewing the tonkeeper-ton repo, I assembled another test branch for review.

It seems they are building addresses differently. Both methods result in a working script deployment. Maybe we can stick to this one as expected by Tonkeeper and other wallets.

I think it’s better to keep V5R1 and rename it to V5Beta as a recommendation. We just need to communicate that anyone who built a script on the V5 Beta contract will need to adjust that one line of code in their codebase. It will create more confusion in the community if we choose to go with V5R2 now.

Totemancer@a1fa67f#diff-690fe05c81bdf07d34a3afd3b777e8f0c10c3b6bc2ec9487b11f989898770a51R46

go get -u github.com/Totemancer/tonutils-go@e041724db2d3cb52d2c82340a9268afdb3d8a539

@Totemancer
Copy link
Contributor Author

Totemancer commented Jul 5, 2024

It will be easier to decide and adjust the code after reviewing tongo new commits.

Official V5R1 Implementation: tonkeeper/tongo@6ec6aa6

@Totemancer
Copy link
Contributor Author

Actually, there’s no need to overcomplicate things. We can hardcode the new wallet IDs according to this scheme:

tonkeeper/tonkeeper-ton@d9aec6a#diff-c8ee60dec2f4e3ee55ad5e40f56fd9a104f21df78086a114d33d62e4fa0ffee6R149

@Totemancer
Copy link
Contributor Author

Finished the integration according to the Tonkeeper integration.

Kept the name as V5R1 as that will be the name of this contract. We do not want to confuse future users with V5R2, as it might be a different revision down the line.

To test from the master branch:
go get -u github.com/Totemancer/tonutils-go@f14e3612e7e5748f3afd28daa7d37a44dbbf494d

We can wait a few days until it appears in wallets to compare addresses, then merge. But it should be good to go now.​

@Totemancer
Copy link
Contributor Author

Totemancer commented Jul 6, 2024

Compared results with the tonkeeper-ton TypeScript library.

let wallet = WalletContractV5R1.create({ publicKey: keyPair.publicKey });

Good news! The wallet address matches! The integration is good. We can wait a couple of days to make sure Wallet V5 is finalized. There are a couple of open pull requests https://github.com/ton-blockchain/wallet-contract-v5/pulls about proposed changes (minor). These will be finalized in a few days max.

Let me know your decision on the wallet naming discussed above.

@Totemancer
Copy link
Contributor Author

Now, explorers on this contract are reporting v5r1. Arriving early next week, we can have this ready for release day.

Official naming as follows: tonkeeper/tonkeeper-web@2b26520#diff-a566641f3f297280b5460b335bf1cb1aabfdb583b75957c3c9132e1d372289beR15

@xssnick xssnick changed the base branch from master to dev-v19 July 18, 2024 12:43
@xssnick xssnick merged commit 59f4297 into xssnick:dev-v19 Jul 18, 2024
1 check failed
@xssnick
Copy link
Owner

xssnick commented Jul 18, 2024

Thank you, merged, I did small naming refactoring, because if we make V5R1 with new code, users who used V5R1 previously could get a problems after upgrade (wallet address will be changed silently), so I renamed both of them, to V5R1Beta and V5R1Final

@Totemancer
Copy link
Contributor Author

Thank you, merged, I did small naming refactoring, because if we make V5R1 with new code, users who used V5R1 previously could get a problems after upgrade (wallet address will be changed silently), so I renamed both of them, to V5R1Beta and V5R1Final

Yes, it makes sense. Agreed.
Not a problem - people can change that one line of code in their libraries to continue using Beta or switch to Final.

Using the merged V5R1 codes for 2 weeks now and it has been rock solid so far. Thank you for merging.
Hopefully, we can contribute to the library in the future too!

Thank you for your work.

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