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

feat(stdlib): Slice.loadBasechainAddress() and Slice.skipBasechainAddress() #2395

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

Conversation

novusnota
Copy link
Member

@novusnota novusnota commented Mar 17, 2025

Issue

Closes #2412

@novusnota novusnota added this to the v1.6.4 milestone Mar 17, 2025
@novusnota novusnota changed the title feat(docs/stdlib): document BasechainAddress struct, related functions, and add Slice.loadBasechainAddress(), Slice.skipBasechainAddress() feat(docs/stdlib): BasechainAddress struct and related functions, plus Slice.loadBasechainAddress(), Slice.skipBasechainAddress() Mar 17, 2025
Copy link
Member

@anton-trunov anton-trunov left a comment

Choose a reason for hiding this comment

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

Let's remove all the new functions and keep just the docs changes

@novusnota novusnota changed the title feat(docs/stdlib): BasechainAddress struct and related functions, plus Slice.loadBasechainAddress(), Slice.skipBasechainAddress() feat(docs): BasechainAddress struct and related functions Mar 17, 2025
@anton-trunov
Copy link
Member

I don't understand the purpose of this PR now, if we remove the new functions. If you want to fix the links, please open the new one

@novusnota novusnota changed the title feat(docs): BasechainAddress struct and related functions feat(stdlib): Slice.loadBasechainAddress() and Slice.skipBasechainAddress() Mar 18, 2025
@novusnota novusnota reopened this Mar 18, 2025
@novusnota novusnota marked this pull request as ready for review March 18, 2025 08:57
@novusnota novusnota requested a review from a team as a code owner March 18, 2025 08:57
@novusnota
Copy link
Member Author

novusnota commented Mar 18, 2025

I don't understand the purpose of this PR now, if we remove the new functions. If you want to fix the links, please open the new one

(which is why i don't like posting incomplete drafts)

  1. this pr is now with functions
  2. and with important fixes
  3. but not all, since those were meant to be done for basechain address too, but now its docs are in another pr feat(docs): add documentation for basechain address related struct and functions #2411

@novusnota
Copy link
Member Author

novusnota commented Mar 18, 2025

Now this needs #2411 because of the reference of BasechainAddress, which was supposed to be in core-addresses :)

@anton-trunov
Copy link
Member

(which is why i don't like posting incomplete drafts)

there is a difference between drafts and mixing new features and fixes together

Comment on lines +1209 to +1211
Attempts to load such [`BasechainAddress{:tact}`][base-addr] struct when [`Slice{:tact}`][slice] doesn't contain it throw an exception with [exit code 8](/book/exit-codes#8): `Cell overflow`.

Attempts to load more data than [`Slice{:tact}`][slice] contains throw an exception with [exit code 9](/book/exit-codes#9): `Cell underflow`.
Copy link
Member

Choose a reason for hiding this comment

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

What happens when a slice that is not a valid basechain addess is passed?

Copy link
Member Author

@novusnota novusnota Mar 18, 2025

Choose a reason for hiding this comment

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

Exit code 9.

Actually, I may have found a bunch of long-standing typos — the exit code 8 doesn't seem right for the .load functions.

Double-checking those now.

UPD: They're incorrect

Copy link
Member

Choose a reason for hiding this comment

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

This should be documented

}

// addr_std, skip the remaining prefix bits
self.skipBits(9);
Copy link
Member

Choose a reason for hiding this comment

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

what happens when we have workchains other than basechain?

@anton-trunov anton-trunov modified the milestones: v1.6.4, v1.6.5 Mar 18, 2025
@novusnota
Copy link
Member Author

novusnota commented Mar 18, 2025

Agree with moving it to 1.6.5, we can be more specific when attempting to load the BasechainAddress from a Slice. Like, to use exit codes 138 (not a basechain address), for example.

Plus, the exit code 8 doesn't seem achievable with the .load instructions, so that has to be fixed (in progress)

@anton-trunov anton-trunov removed this from the v1.6.5 milestone Mar 18, 2025
@novusnota novusnota marked this pull request as draft March 20, 2025 01:29
@novusnota
Copy link
Member Author

Converting to a draft until learnXinY is ready for review. Then, I'll return to this.

Here, I'll:

  1. improve the contents of two new load/skip functions and make them use the basechain-related exit codes
  2. make all .store functions mention the exit code 8 and all .load/.skip functions not mention it

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.

Add utility functions for dealing with BasechainAddresses for a Slice
2 participants