Skip to content

SDK broadcast timeout separated#762

Merged
feruzm merged 3 commits into
developfrom
broad
Apr 20, 2026
Merged

SDK broadcast timeout separated#762
feruzm merged 3 commits into
developfrom
broad

Conversation

@feruzm
Copy link
Copy Markdown
Member

@feruzm feruzm commented Apr 20, 2026

Summary by CodeRabbit

  • New Features

    • Added an async broadcast function for non-blocking transaction submission that returns a transaction id immediately (block confirmation handled separately).
  • Improvements

    • Separated timeout settings for read and broadcast operations to improve reliability and clarity when interacting with the network.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 20, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 769be0ea-3b4d-4e04-b263-0a90ca0f144d

📥 Commits

Reviewing files that changed from the base of the PR and between 32bd8e9 and c96a876.

⛔ Files ignored due to path filters (13)
  • packages/sdk/dist/browser/hive.js is excluded by !**/dist/**
  • packages/sdk/dist/browser/hive.js.map is excluded by !**/dist/**, !**/*.map
  • packages/sdk/dist/browser/index.d.ts is excluded by !**/dist/**
  • packages/sdk/dist/browser/index.js is excluded by !**/dist/**
  • packages/sdk/dist/browser/index.js.map is excluded by !**/dist/**, !**/*.map
  • packages/sdk/dist/node/hive.cjs is excluded by !**/dist/**
  • packages/sdk/dist/node/hive.cjs.map is excluded by !**/dist/**, !**/*.map
  • packages/sdk/dist/node/hive.mjs is excluded by !**/dist/**
  • packages/sdk/dist/node/hive.mjs.map is excluded by !**/dist/**, !**/*.map
  • packages/sdk/dist/node/index.cjs is excluded by !**/dist/**
  • packages/sdk/dist/node/index.cjs.map is excluded by !**/dist/**, !**/*.map
  • packages/sdk/dist/node/index.mjs is excluded by !**/dist/**
  • packages/sdk/dist/node/index.mjs.map is excluded by !**/dist/**, !**/*.map
📒 Files selected for processing (5)
  • packages/sdk/CHANGELOG.md
  • packages/sdk/package.json
  • packages/sdk/src/modules/core/hive-tx.ts
  • packages/wallets/CHANGELOG.md
  • packages/wallets/package.json

📝 Walkthrough

Walkthrough

Separated read and broadcast timeouts, switched broadcast RPC calls to use the new broadcast timeout, and added an async broadcast helper that signs a transaction and broadcasts it non-synchronously (mempool acceptance) returning a BroadcastResult promise.

Changes

Cohort / File(s) Summary
Timeout Configuration
packages/sdk/src/hive-tx/config.ts
Changed timeout to explicitly mean read-API timeout and added broadcastTimeout: 15_000.
Broadcast RPC Integration
packages/sdk/src/hive-tx/helpers/call.ts
Updated callRPCBroadcast default timeout to config.broadcastTimeout instead of config.timeout.
Async Broadcast Function
packages/sdk/src/modules/core/hive-tx.ts
Added exported broadcastOperationsAsync(ops, key) that signs operations and calls tx.broadcast(false), returning Promise<BroadcastResult>.
Packaging / Changelogs
packages/sdk/CHANGELOG.md, packages/sdk/package.json, packages/wallets/CHANGELOG.md, packages/wallets/package.json
Bumped packages/sdk to 2.2.4, packages/wallets to 4.0.2, and added changelog entries noting the broadcast-timeout change and dependency bump.

Sequence Diagram

sequenceDiagram
    actor User
    participant SDK as SDK
    participant Signer as Signer
    participant Hive as Hive RPC

    User->>SDK: broadcastOperationsAsync(ops, key)
    SDK->>Signer: build & sign Transaction (ops, key)
    Signer-->>SDK: signed Transaction
    SDK->>Hive: tx.broadcast(false) (uses config.broadcastTimeout)
    Hive-->>SDK: Broadcast acceptance / tx_id
    SDK-->>User: Promise<BroadcastResult> (tx_id, status: "unknown")
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A rabbit's rhyme upon these changes made:
Time split tidy, broadcasts slow and sure,
Reads stay brisk, signs hop swift and pure,
Sent to the hive, awaiting distant proof,
I nibble logs and celebrate aloof. 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: separating broadcast timeout configuration from the generic API timeout in the SDK.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch broad

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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/sdk/src/modules/core/hive-tx.ts`:
- Around line 145-158: Update the JSDoc comment above the asynchronous broadcast
helper to clarify that "fire-and-forget" only refers to not waiting for block
inclusion; the helper still awaits the call to broadcast_transaction and can
throw transport/RPC errors. Edit the comment around references to
broadcast_transaction, Transaction.checkStatus(), and broadcastOperations() so
callers know errors from the RPC/transport layer are propagated immediately
while block confirmation must be checked separately.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 137b6144-b783-4c68-9813-32a964f02293

📥 Commits

Reviewing files that changed from the base of the PR and between 1f3578f and 32bd8e9.

⛔ Files ignored due to path filters (15)
  • packages/sdk/dist/browser/hive-CwYfobOc.d.ts is excluded by !**/dist/**
  • packages/sdk/dist/browser/hive.d.ts is excluded by !**/dist/**
  • packages/sdk/dist/browser/hive.js is excluded by !**/dist/**
  • packages/sdk/dist/browser/hive.js.map is excluded by !**/dist/**, !**/*.map
  • packages/sdk/dist/browser/index.d.ts is excluded by !**/dist/**
  • packages/sdk/dist/browser/index.js is excluded by !**/dist/**
  • packages/sdk/dist/browser/index.js.map is excluded by !**/dist/**, !**/*.map
  • packages/sdk/dist/node/hive.cjs is excluded by !**/dist/**
  • packages/sdk/dist/node/hive.cjs.map is excluded by !**/dist/**, !**/*.map
  • packages/sdk/dist/node/hive.mjs is excluded by !**/dist/**
  • packages/sdk/dist/node/hive.mjs.map is excluded by !**/dist/**, !**/*.map
  • packages/sdk/dist/node/index.cjs is excluded by !**/dist/**
  • packages/sdk/dist/node/index.cjs.map is excluded by !**/dist/**, !**/*.map
  • packages/sdk/dist/node/index.mjs is excluded by !**/dist/**
  • packages/sdk/dist/node/index.mjs.map is excluded by !**/dist/**, !**/*.map
📒 Files selected for processing (3)
  • packages/sdk/src/hive-tx/config.ts
  • packages/sdk/src/hive-tx/helpers/call.ts
  • packages/sdk/src/modules/core/hive-tx.ts

Comment thread packages/sdk/src/modules/core/hive-tx.ts
@feruzm feruzm added the patch Bug fixes and patches (1.0.0 → 1.0.1), add this only if any packages/ have patch changes in PR label Apr 20, 2026
@feruzm feruzm merged commit b324acc into develop Apr 20, 2026
1 check was pending
@feruzm feruzm deleted the broad branch April 20, 2026 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch Bug fixes and patches (1.0.0 → 1.0.1), add this only if any packages/ have patch changes in PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant