fix(tvm): never lose the txid on a Tron broadcast failure

[droplet-rl]

arch/tvm/submitTransaction() signs before it broadcasts, so the txid is known locally before sendRawTransaction() is called. Two exits threw it away. Tron has no nonce, so a resubmission is a second, independent transaction — the txid is the only handle a caller has, and losing it is a double-execution risk.

1. A throw from the send discarded the txid. A transport error or timeout doesn't prove the node refused the transaction. The send now rethrows a TronBroadcastError carrying txid, so the caller can verify on-chain instead of resubmitting.
2. DUP_TRANSACTION_ERROR was reported as a failed send. That's Tron's "already known" — the node holds this transaction. Now a successful send with the txid, as the EVM path does.
3. Rejections carry code and message. A bare boolean can't distinguish a definite rejection (TAPOS_ERROR) from an ambiguous one (SERVER_BUSY). message is utf8-decoded, since Tron hex-encodes it.
4. The native-transfer path had the same defect via trx.sendTransaction(), which fuses build/sign/broadcast. Now three steps, sharing the broadcast handling.

Notes:

• TronBroadcastError ships with isTronBroadcastError(). Prefer the guard over instanceof: the CJS and ESM builds don't share class identity.
• Success shape is unchanged and code/message are omitted when absent, so existing result assertions hold. The behavioural changes are the new throw type and the duplicate-as-success verdict.
• Not included: a table of which response_code values prove a transaction is not on-chain. Exposing code leaves that policy to the caller; say the word if it belongs here.

Testing: test/Tvm.TransactionUtils.test.ts, 13 passing — txid surviving a throw and duplicate-as-success on both the call and transfer paths, decoded code/message, and the type guard.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-08T16:58:17.363031Z	2f3ffcf	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2f3ffcf937

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Reject fractional transfer amounts before sendTrx

When callValue is a positive non-integer number of SUN, this path no longer preserves the validation performed by the previous trx.sendTransaction call. TronWeb's transactionBuilder.sendTrx applies parseInt(amount) before validating it, so a value such as 1.9 is silently converted to 1 and broadcast instead of being rejected, causing the recipient to receive a different amount than requested. Validate that amount is an integer before calling the builder.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Handle numeric duplicate response codes

When a provider returns the numeric response-code form declared by TronWeb's Types.BroadcastReturn (DUP_TRANSACTION_ERROR is enum value 5), converting it with String produces "5", so this comparison cannot recognize the duplicate. The exact same transaction is then reported as failed, potentially prompting the unsafe rebuild/resubmission this change is intended to prevent; normalize the numeric enum to its name or explicitly accept value 5.

Useful? React with 👍 / 👎.

[droplet-rl]

I've used my 1 automated round on this PR, so I'm ignoring further automated-reviewer activity. To re-activate me: have a human @-mention @droplet-rl in a comment (resets the budget), or assign me to the PR (raises the budget to 3 rounds).

[droplet-rl]

Both findings are real and are fixed in 884a663. I checked each against the vendored tronweb 6.2.2 source rather than taking them at face value — the first is a genuine regression I introduced, the second is a latent gap.

1. Fractional transfer amounts — confirmed regression. The claim holds exactly as described. trx.sendTransaction, which this PR replaced on the transfer path, guards up front (src/lib/trx.ts:832):

if (!utils.isInteger(amount) || amount <= 0) throw new Error('Invalid amount provided');

whereas transactionBuilder.sendTrx reassigns before it validates, so the integer check is tautological:

async sendTrx(to, amount = 0, ...) {
  amount = parseInt(amount);
  this.validator.notValid([..., { name: 'amount', type: 'integer', gt: 0, value: amount }]);

So 1.9 SUN passed validation as 1 and got broadcast — the recipient short-changed, silently. Guard restored in transferNative.

Worth noting the blast radius is narrower than it first looks, and only in one direction: the contract-call path never had this problem. triggerSmartContract validates callValue as { type: 'integer', gte: 0 } with no preceding parseInt, so a fractional callValue there is rejected by TronWeb itself. The refactor introduced an asymmetry between the two paths; the guard removes it.

2. Numeric response codes — accepted, with a caveat on reachability. BroadcastReturn.code is typed BroadcastReturn_response_code, a numeric enum with DUP_TRANSACTION_ERROR = 5 (src/types/Trx.ts:180), so String(5) === "5" would indeed have missed the duplicate and reported the node's own copy of the transaction as a failed send — the exact double-execution this PR exists to prevent.

Being straight about what I could and couldn't verify: I found no provider that actually returns the numeric form. A TRON HTTP node returns the name, which is why the original code compared against names and why the tests passed. So this was a latent gap against a shape TronWeb's own typings declare, not a live bug. I still took it — the fix is a lookup table, and the cost of being wrong is unbounded.

Rather than special-case 5, I resolve any ordinal back to its name, so code reaches callers in one form regardless of which the provider used:

• numeric 5 and string "5" both normalise to DUP_TRANSACTION_ERROR
• 6 → TAPOS_ERROR, and so on across the enum
• an unrecognised code is passed through rather than dropped — if the node said something, a caller debugging a rejection should see it

The table mirrors BroadcastReturn_response_code, which tronweb does not export (hence the copy, with the source cited in a comment). These are protocol-level constants from the Tron protobuf, so drift risk is low.

Verification: 17 passing in test/Tvm.TransactionUtils.test.ts (13 before, +4: fractional-amount rejection, numeric dup code, ordinal→name resolution, unrecognised-code passthrough). yarn lint-check and all three tsc build targets clean.

One process note: a canned "automated round used" comment went out from my account at 16:58 as the review landed — that fired on a timer before I'd read the findings and doesn't reflect my assessment. This reply supersedes it. That said, the round budget is genuinely spent, so I won't respond to further automated activity here; if you want another pass, assign me to the PR.

[droplet-rl]

🔎 View trace

[pxrl]

If these fields are absent on success, should we instead return a discriminated type on failure vs. success?

[droplet-rl]

Agreed — done in 832e5bf.

export interface TronTransactionSuccess {
  txid: string;
  result: true;
}

export interface TronTransactionFailure {
  txid: string;
  result: false;
  code?: string;
  message?: string;
}

export type TronTransactionResult = TronTransactionSuccess | TronTransactionFailure;

txid stays common to both branches — it is fixed at signing and reported whichever way the node decides, which is the whole point of the PR.

I checked the narrowing rather than assuming it, since the payoff here is entirely compile-time:

error TS2339: Property 'code' does not exist on type 'TronTransactionResult'.      // unnarrowed
error TS2339: Property 'code' does not exist on type 'TronTransactionSuccess'.     // if (r.result)

and if (!r.result) { r.code } resolves to string | undefined.

Both branches are exported so a consumer can name the one it handles rather than reaching for Extract<...>.

Two notes on the shape:

• code and message stay optional on the failure branch. A node need not send either, so making them required would be a lie one level down; a rejection is still only ever inferred from result: false, never from their absence.
• On blast radius: nothing in-repo consumes this type, and code/message were introduced by this PR, so no existing reader gets narrowed. The one theoretical break is a caller constructing a TronTransactionResult with a non-literal result: boolean — that no longer assigns. Worth knowing, but there is no such caller today.

I also added a test that reads code through the narrowing. It is really a compile-time assertion wearing a runtime test's clothes: flattening the union back to one optional-field interface would still satisfy the deep.equal assertions in the neighbouring tests, but would stop that one from type-checking.

One thing I deliberately left alone, since it is adjacent to your question rather than in it. A DUP_TRANSACTION_ERROR rejection is mapped to { txid, result: true }, so the success branch cannot express "the node already had this". I think that is right — the contract is "the node holds this txid", and a duplicate satisfies it — but it does mean a caller cannot distinguish a fresh accept from a duplicate. Adding duplicate?: true to the success branch later is a non-breaking change, so I would rather not guess at it now. Say the word if you want it.

[droplet-rl]

Thanks @pxrl — good call, and it's in as 832e5bf. Replied on the thread with the details; summary here.

TronTransactionResult is now a discriminated union on result, with both branches exported:

export interface TronTransactionSuccess { txid: string; result: true }
export interface TronTransactionFailure { txid: string; result: false; code?: string; message?: string }
export type TronTransactionResult = TronTransactionSuccess | TronTransactionFailure;

txid is common to both — fixed at signing, reported whichever way the node decides, which is the point of the PR. I verified the narrowing rather than assuming it, since the payoff is entirely compile-time: unnarrowed .code and .code on the success branch are both now type errors, and if (!r.result) resolves code to string | undefined. Added a test that reads code through the narrowing, which would stop compiling if the union were ever flattened back — the neighbouring deep.equal assertions wouldn't catch that.

Safe to tighten: nothing in-repo consumes the type, and code/message were introduced by this PR, so no existing reader is affected.

Two things I flagged on the thread rather than deciding unilaterally:

• code/message remain optional on the failure branch — a node need not send either, so requiring them would just move the lie down a level.
• A DUP_TRANSACTION_ERROR rejection maps to { txid, result: true }, so the success branch can't express "the node already had this". I think that's correct (the contract is "the node holds this txid"), but it does mean a caller can't tell a fresh accept from a duplicate. duplicate?: true would be a non-breaking addition later, so I left it out rather than guess.

Verification: 18 passing in test/Tvm.TransactionUtils.test.ts, yarn lint-check clean, all three tsc targets clean.

Also worth surfacing since it's easy to miss in the commit list: the automated reviewer caught a real regression I'd introduced, fixed in 884a663. transferNative had replaced trx.sendTransaction (which rejects a non-integer amount) with transactionBuilder.sendTrx, which parseInts the amount before validating it — so a fractional value was truncated and broadcast rather than rejected, short-changing the recipient. Guard restored; the contract-call path was never affected, as TronWeb validates callValue as an integer there. That commit also resolves numeric response_code ordinals to their names, so a provider returning the enum form (DUP_TRANSACTION_ERROR = 5) can't have a duplicate misread as a failed send.

[droplet-rl]

🔎 View trace

[pxrl]

Are these still optional even when returning a discriminated failure type?

[droplet-rl]

Good push — you were right to ask twice. My previous answer ("a node need not send one") was accurate but it was defending a weak type instead of asking why the weak case existed. Digging into it turned up a bug. code is now required, in d312788.

The reason a code-less failure was representable at all is that sendRawTransaction returns the HTTP body verbatim:

const result = await this.tronWeb.fullNode.request<...>('wallet/broadcasttransaction', signedTransaction, 'post');
return { ...result, transaction: signedTransaction };

No normalisation, and notably no Error check — TronWeb does guard if (transaction.Error) throw ... on its transaction-building paths (helper.ts:17, TransactionBuilder.ts:1075, trx.ts:1549) but not here. So TRON's bare { Error: "..." } shape, an empty body, or anything a proxy mangles all reach us as result falsy with no response_code.

The type question turned out to be the smaller half of it. None of those responses establish that the transaction was rejected. The node may have accepted it and failed to say so. Reporting result: false there asserts a verdict we do not have, and on TRON a false "it failed" invites the resubmit that — with no nonce to replace through — executes a second time. That is the exact hazard this PR exists to close, and the original code walked into it one branch over from where I was looking.

So rather than make code required by inventing a filler value (OTHER_ERROR would have been actively harmful — indistinguishable from the node actually saying OTHER_ERROR), I routed the case to the error that already means "may or may not have been sent" and carries the txid:

if (!isDefined(code)) {
  throw new TronBroadcastError(
    `TRON broadcast for ${txID} returned no response code, so its outcome is unknown: ${describeCodelessRejection(broadcast)}`,
    txID
  );
}

describeCodelessRejection surfaces the Error string so the diagnostic is not swallowed. Three outcomes now, and only the middle one is safe to retry blindly: accepted (including a duplicate the node already holds), definitely rejected with a reason, unknown.

With that, a failure is only ever constructed when the node named a reason, so code: string is a guarantee rather than a hope. Verified, since the payoff is compile-time:

• if (!r.result) { const code: string = r.code } — compiles clean
• const message: string = r.message — still error TS2322: Type 'string | undefined' is not assignable

message I did keep optional, and I want to be explicit that this is a considered split rather than the same hand-wave as before: the protocol's Return.message is genuinely empty for some rejections, and it is human prose, not something to branch on — code is the machine-readable half, and that is the one now guaranteed.

Behavioural change worth flagging on review: a { result: false } response with no code used to return; it now throws. Only reachable via malformed/proxy responses, and callers already have to handle TronBroadcastError, but it is a contract change rather than pure tightening. Test updated accordingly, plus new coverage for the code-less and bare-Error shapes. 20 passing, lint and all three tsc targets clean.

[droplet-rl]

Thanks @pxrl — asking that a second time was the right call. It shook out a bug, not just a type tweak. d312788; details on the thread, summary here.

My last answer ("a node need not send a code") was true but was defending a weak type rather than asking why the weak case existed. Chasing that down: sendRawTransaction returns the HTTP body verbatim — no normalisation, and no Error check, even though TronWeb does guard if (transaction.Error) throw ... on its transaction-building paths (helper.ts:17, TransactionBuilder.ts:1075, trx.ts:1549). So TRON's bare { Error: "..." } shape, an empty body, or anything a proxy mangles all arrived as result falsy with no response_code.

The typing was the smaller half. None of those responses prove a rejection — the node may have taken the transaction and failed to say so. Reporting result: false asserted a verdict we don't have, and on TRON a false "it failed" invites the resubmit that executes a second time. That's the exact hazard this PR exists to close, sitting one branch over from where I'd been looking.

Fix: route the code-less case to TronBroadcastError, which already means "may or may not have been sent" and carries the txid, surfacing the node's Error string as the diagnostic. Three outcomes now, only the middle safe to retry blindly — accepted (incl. a duplicate), definitely rejected with a reason, unknown.

That makes code: string a guarantee rather than a hope, so it's now required. Verified at compile time: const code: string = r.code compiles after narrowing; const message: string = r.message still errors. I deliberately did not make it required by inventing a filler — OTHER_ERROR would be indistinguishable from the node actually saying OTHER_ERROR.

message stays optional, and this one is a considered split rather than a repeat of the hand-wave: the protocol's Return.message is genuinely empty for some rejections, and it's human prose. code is the machine-readable half, and that's the half now guaranteed.

Please flag if you'd rather not take this: a { result: false } response with no code used to return and now throws. Only reachable via malformed or proxied responses, and callers already handle TronBroadcastError, but it's a contract change rather than pure tightening — so it's your call, not mine.

Verification: 20 passing in test/Tvm.TransactionUtils.test.ts (18 → 20; updated the code-less test, added coverage for the code-less and bare-Error shapes), yarn lint-check clean, all three tsc targets clean.

[droplet-rl]

🔎 View trace