Skip to content

feat(ton): lite-server connection tweaks#3697

Merged
swift1337 merged 1 commit intodevelopfrom
feat/ton/lite-client-config
Mar 12, 2025
Merged

feat(ton): lite-server connection tweaks#3697
swift1337 merged 1 commit intodevelopfrom
feat/ton/lite-client-config

Conversation

@swift1337
Copy link
Contributor

@swift1337 swift1337 commented Mar 12, 2025

Related: #2810

Summary by CodeRabbit

  • Chores

    • Upgraded an external dependency to a newer version, incorporating improvements and fixes.
  • New Features

    • Refined connection management settings to dynamically adjust the number of active connections and allocate dedicated processing workers, enhancing request handling and overall responsiveness for users.

@swift1337 swift1337 added no-changelog Skip changelog CI check TON_TESTS Runs TON E2E tests labels Mar 12, 2025
@swift1337 swift1337 self-assigned this Mar 12, 2025
@swift1337 swift1337 requested a review from a team as a code owner March 12, 2025 13:24
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 12, 2025

📝 Walkthrough

Walkthrough

The changes update the dependency version in the project and modify the client instantiation logic for TON lite API communication. In the go.mod file, the dependency github.com/tonkeeper/tongo is upgraded from version v1.9.3 to v1.14.8. In the client.go file, the NewFromSource function has been altered to remove the static liteapi.WithDetectArchiveNodes() argument and introduce dynamic connection parameters—computing the maximum number of connections based on the lite server list and specifying a fixed number of workers per connection.

Changes

File(s) Changed Change Summary
go.mod Updated dependency github.com/tonkeeper/tongo from version v1.9.3 to v1.14.8.
zetaclient/.../client.go Modified NewFromSource by removing liteapi.WithDetectArchiveNodes() and adding liteapi.WithMaxConnectionsNumber(len(cfg.LiteServers)) and liteapi.WithWorkersPerConnection(workers) (with a fixed value of 8).

Sequence Diagram(s)

sequenceDiagram
    participant C as Caller
    participant NS as NewFromSource
    participant L as liteapi.NewClient

    C->>NS: Invoke NewFromSource(ctx, urlOrPath)
    NS->>L: Call liteapi.NewClient with dynamic parameters:
    note right of NS: - MaxConnectionsNumber based on cfg.LiteServers\n- WorkersPerConnection set to 8
    L-->>NS: Return instantiated client
    NS-->>C: Return client instance
Loading
✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov
Copy link

codecov bot commented Mar 12, 2025

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 64.58%. Comparing base (5bac6f9) to head (7cd4d09).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
zetaclient/chains/ton/liteapi/client.go 0.00% 5 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3697      +/-   ##
===========================================
- Coverage    64.59%   64.58%   -0.01%     
===========================================
  Files          469      469              
  Lines        32911    32915       +4     
===========================================
  Hits         21259    21259              
- Misses       10687    10691       +4     
  Partials       965      965              
Files with missing lines Coverage Δ
zetaclient/chains/ton/liteapi/client.go 11.56% <0.00%> (-0.33%) ⬇️
🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🔭 Outside diff range comments (1)
zetaclient/chains/ton/liteapi/client.go (1)

40-62: 🛠️ Refactor suggestion

Add unit tests for the new connection configuration.

The static analysis shows that the new connection configuration code isn't covered by tests. Consider adding unit tests that verify the client is created with the expected number of connections and workers, especially since this is now dynamically calculated based on configuration.

 // NewFromSource creates a new client from a URL or a file path.
 func NewFromSource(ctx context.Context, urlOrPath string) (*Client, error) {
 	cfg, err := config.FromSource(ctx, urlOrPath)
 	if err != nil {
 		return nil, errors.Wrap(err, "unable to get config")
 	}
 
 	// most likely, cfg would contain a single lite server (or a small group)
 	// thus we want several connection per lite-server to speed up the requests
 	const workers = 8
 
 	client, err := liteapi.NewClient(
 		liteapi.WithConfigurationFile(*cfg),
 		liteapi.WithMaxConnectionsNumber(len(cfg.LiteServers)),
 		liteapi.WithWorkersPerConnection(workers),
 	)
 
 	if err != nil {
 		return nil, errors.Wrap(err, "unable to create client")
 	}
 
 	return New(client), nil
 }

You should add a test similar to:

func TestNewFromSource(t *testing.T) {
    // Setup mock context and config
    ctx := context.Background()
    mockCfg := &config.Config{
        LiteServers: []config.LiteServer{
            {/* server details */},
            {/* server details */},
        },
    }
    
    // Mock the config.FromSource function
    origFromSource := config.FromSource
    config.FromSource = func(ctx context.Context, urlOrPath string) (*config.Config, error) {
        return mockCfg, nil
    }
    defer func() { config.FromSource = origFromSource }()
    
    // Create a mock for liteapi.NewClient to verify the parameters
    var capturedMaxConn, capturedWorkers int
    origNewClient := liteapi.NewClient
    liteapi.NewClient = func(opts ...liteapi.ClientOption) (*liteapi.Client, error) {
        // Extract and verify the parameters from the options
        // This would require additional test infrastructure to inspect the options
        capturedMaxConn = len(mockCfg.LiteServers)
        capturedWorkers = 8
        return &liteapi.Client{}, nil
    }
    defer func() { liteapi.NewClient = origNewClient }()
    
    // Call the function
    client, err := NewFromSource(ctx, "test-path")
    
    // Verify results
    assert.NoError(t, err)
    assert.NotNil(t, client)
    assert.Equal(t, len(mockCfg.LiteServers), capturedMaxConn)
    assert.Equal(t, 8, capturedWorkers)
}
🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 49-50: zetaclient/chains/ton/liteapi/client.go#L49-L50
Added lines #L49 - L50 were not covered by tests


[warning] 53-54: zetaclient/chains/ton/liteapi/client.go#L53-L54
Added lines #L53 - L54 were not covered by tests


[warning] 56-56: zetaclient/chains/ton/liteapi/client.go#L56
Added line #L56 was not covered by tests

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5bac6f9 and 7cd4d09.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (2)
  • go.mod (1 hunks)
  • zetaclient/chains/ton/liteapi/client.go (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to ...

**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

  • zetaclient/chains/ton/liteapi/client.go
🪛 GitHub Check: codecov/patch
zetaclient/chains/ton/liteapi/client.go

[warning] 49-50: zetaclient/chains/ton/liteapi/client.go#L49-L50
Added lines #L49 - L50 were not covered by tests


[warning] 53-54: zetaclient/chains/ton/liteapi/client.go#L53-L54
Added lines #L53 - L54 were not covered by tests


[warning] 56-56: zetaclient/chains/ton/liteapi/client.go#L56
Added line #L56 was not covered by tests

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: start-ton-test / e2e
  • GitHub Check: start-e2e-test / e2e
🔇 Additional comments (1)
go.mod (1)

313-313:

Details

❓ Verification inconclusive

Verify compatibility with the significant tongo library version upgrade.

The update from an unspecified version to v1.14.8 of the github.com/tonkeeper/tongo dependency represents a substantial version jump. This update likely supports the connection configuration changes in the client code, but may introduce API changes or deprecations that should be carefully validated.


🏁 Script executed:

#!/bin/bash
# Check the changelog or release notes for the tongo library to understand what changed
# between the previous version and v1.14.8

# First, find the previous version that was being used
grep -A 2 "github.com/tonkeeper/tongo" go.mod.bak 2>/dev/null || echo "Previous version not found in backup"

# Look for GitHub release notes or changelog
curl -s "https://api.github.com/repos/tonkeeper/tongo/releases" | grep -m 5 "tag_name\|body" | sed 's/"tag_name":/\nVersion:/g' | sed 's/"body":/\nChanges:/g' | sed 's/\\r\\n/\n/g'

Length of output: 397


Compatibility Verification Required for tongo v1.14.8 Upgrade

After reviewing the updated dependency and our investigation:

  • The previous version could not be retrieved from the backup, and the GitHub releases endpoint shows older versions (e.g., v1.9.3 and v1.1.1) with no detailed change logs.
  • There is no clear changelog or documented migration guide directly addressing the jump to v1.14.8.
  • Given the significant version upgrade, it is advisable to manually verify that the new API methods and connection configuration changes in the client code are fully compatible.

Action Items:

  • Confirm via the upstream repository or commit history for any breaking API changes introduced in v1.14.8.
  • Validate that all uses of the tongo library within the client have been adjusted if necessary.
  • Strengthen the test coverage around the connection configuration to catch any subtle incompatibilities.

@swift1337 swift1337 added this pull request to the merge queue Mar 12, 2025
Merged via the queue into develop with commit 4ce9ef0 Mar 12, 2025
48 of 49 checks passed
@swift1337 swift1337 deleted the feat/ton/lite-client-config branch March 12, 2025 17:15
swift1337 added a commit that referenced this pull request Mar 12, 2025
* feat(ton): lite-server connection tweaks (#3697) [backport]

* feat(ton): send real gas usage (#3699) [backport]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Skip changelog CI check TON_TESTS Runs TON E2E tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants