Skip to content

fix: increased server timeout#280

Merged
guglez merged 7 commits into
mainfrom
fix/sadiq/server-timeout
May 22, 2026
Merged

fix: increased server timeout#280
guglez merged 7 commits into
mainfrom
fix/sadiq/server-timeout

Conversation

@sadiq1971
Copy link
Copy Markdown
Member

No description provided.

sadiq1971 added 7 commits May 6, 2026 23:45
…tstrap

Switch from CIP56TransferFactory (atomic, local-only) to DA's AllocationFactory
from utility_registry_app_v0. This factory creates TransferOffer contracts on
TransferFactory_Transfer, matching devnet behaviour where the receiver must
exercise TransferInstruction_Accept to complete the transfer.

Also bootstraps TransferRule and InstrumentConfiguration (with empty
holderRequirements) which are required as choiceContextData when the receiver
accepts the transfer offer.

Closes #258
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request increases server and RPC timeouts to 60 seconds across several configuration files. It also includes a significant refactor of the registry API integration to align with the Splice token-standard spec, updating request/response structures and ensuring consistent timestamp handling between registry calls and on-ledger transactions. Feedback highlights that the PR description should be updated to reflect this refactor, suggests defining a named constant for the ledger time skew offset, and recommends a performance optimization for JSON null checks to avoid unnecessary heap allocations.

I am having trouble creating individual review comments. Click here to see my feedback.

pkg/cantonsdk/token/client.go (911)

high

The pull request title and description only mention increasing server timeouts, but the changes include a significant refactor of the registry API integration (aligning with the Splice token-standard spec). This mismatch makes the commit history misleading. Please update the PR title and description to accurately reflect the scope of these changes, including the breaking changes to the registry request/response structures.

pkg/cantonsdk/token/client.go (890)

medium

The backdating offset of -5 * time.Second is used here and in buildTransferCommand (line 647) to account for ledger time skew. This magic number should be defined as a named constant at the package level to ensure consistency and improve maintainability, especially since it directly affects command acceptance on the Canton ledger.

pkg/cantonsdk/token/registry_client.go (168)

medium

Converting json.RawMessage to a string for comparison with jsonNull ("null") causes an unnecessary heap allocation. It is more efficient to use bytes.Equal for this check.

if len(wire.ChoiceContext) > 0 && !bytes.Equal(wire.ChoiceContext, []byte(jsonNull)) {

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@04e9300). Learn more about missing BASE report.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #280   +/-   ##
=======================================
  Coverage        ?   30.61%           
=======================================
  Files           ?      130           
  Lines           ?    10056           
  Branches        ?        0           
=======================================
  Hits            ?     3079           
  Misses          ?     6710           
  Partials        ?      267           
Flag Coverage Δ
unittests 30.61% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@guglez guglez enabled auto-merge (squash) May 22, 2026 19:39
@guglez guglez merged commit ef8a122 into main May 22, 2026
3 checks passed
@guglez guglez deleted the fix/sadiq/server-timeout branch May 22, 2026 19:39
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.

3 participants