search

xssnick / tonutils-go

TON SDK Library in pure Golang for interacting with The Open Network ecosystem using native protocols, such as ADNL, RLDP and etc.
Apache License 2.0
464 stars 95 forks source link

Improve wallet cold signing support #179

Closed igor-u410 closed 5 months ago

igor-u410 commented 5 months ago

Unless I'm mistaken, the current cold signing example doesn't work because BuildExternalMessage (where the signing occurs) requires internet access to make API calls.

My proposal includes a few changes:

  1. Introduce BuildExternalMessageOffline, an offline alternative to BuildExternalMessage.
  2. BuildExternalMessageOffline gets a spec from getOfflineSpec, which receives the SeqNo from the caller rather than getting it from the API.
  3. Add TLB tags to for the Wallet.Message type.
  4. A more detailed cold signing example.

I left a few todos since I'm not familiar enough with the codebase to remove or update cases for HighloadV2R2 and HighloadV2Verified. If there's a better way to accomplish what I'm trying to do here without changes to the code, please let me know.

Finally, thanks for all your work on this project! It's a fantastic contribution to the TON ecosystem.

Tests: Locally executed the cold signing example on the TON mainnet workchain with a different private key.

xssnick commented 5 months ago

Hi, thank you for PR!

Actually Specs are flexible and it is possible to set custom seqno source, see this discussion with example. So BuildExternalMessageOffline is not really needed as separate method.

Current offline signing example is really missing this part with custom seqno, it should be definitely added, will add it in the next release.

For the Wallet.Message, it is not a TLB structure, more like type to use with wallet internally, it is because serialization structure may differ between specs, mode is not guaranteed to be present like this.

Finally, thanks for all your work on this project! It's a fantastic contribution to the TON ecosystem.

Thank you, I'm glad that this library is useful 😊

igor-u410 commented 5 months ago

Ah thanks for the explanation!

igor-u410 commented 5 months ago

One more question: BuildExternalMessageForMany still requires internet access to determine if the account needs to be initialized. Is there a way around this as well?

xssnick commented 5 months ago

Hm, actually you are right, sorry, it fetches the block and account now, I forgot about that.

I have an idea how to support it in BuildExternalMessageForMany, will do in the next minor release.

igor-u410 commented 5 months ago

Thanks! I'll close the PR now