Skip to content

Add transaction-with-token-transfer-lookup research command - #562

Merged
gulshngill merged 5 commits into
mainfrom
feat/research-transaction-token-transfer-lookup
Sep 4, 2026
Merged

Add transaction-with-token-transfer-lookup research command#562
gulshngill merged 5 commits into
mainfrom
feat/research-transaction-token-transfer-lookup

Conversation

@gulshngill

Copy link
Copy Markdown
Contributor

Summary

  • add nansen research transaction-with-token-transfer-lookup
  • map transaction, chain, and timestamp options to the public API request
  • enforce timestamp requirements for supported non-EVM chains and add focused documentation and tests

Validation

  • npm test -- src/__tests__/research.test.js src/__tests__/coverage.test.js — 88 passed
  • npm test — 2,548 passed, 2 skipped
  • npm run lint
  • git diff --check main...HEAD

@nansen-pr-reviewer

nansen-pr-reviewer Bot commented Sep 3, 2026

Copy link
Copy Markdown

pr-reviewer Summary for #7aca298

📝 2 findings

Review completed. Please address the findings below.

Findings by Severity

Severity Count
🟡 Medium 1
🔵 Low 1

Review effort: 2/5 (Simple)

Summary

This PR cleanly adds the nansen research transaction-with-token-transfer-lookup command. The implementation follows existing patterns — requireOptions for guard validation, cleanBody to strip undefined fields so block_timestamp is omitted when absent, proper schema.json update, changeset, and README entry. Tests cover dispatch, the timestamp guard, and the missing-hash error. Overall it's a solid addition with two items worth addressing.

Findings

src/api.js — medium

transactionWithTokenTransferLookup is not placed in a section of its own and transaction_hash is sent as undefined when called directly (bypassing the CLI layer)

The method is dropped between addressPerpPnlSummary (inside the implicit profiler section) and the // ============= Token God Mode Endpoints ============= section header with no grouping comment. While the cleanBody helper correctly strips undefined values so the absent hash is not serialised into the JSON body, a caller using api.transactionWithTokenTransferLookup({}) directly will silently fire a request without transaction_hash, relying entirely on the API to reject it. Every other endpoint in the file that has required fields applies client-side validation (e.g. requireValidAddress, or the requireOptions-equivalent at the api layer). Consider adding a guard:

async transactionWithTokenTransferLookup(params = {}) {
  const { chain = 'ethereum', transactionHash, blockTimestamp } = params;
  if (!transactionHash) {
    throw new NansenError('Required: transaction_hash', ErrorCode.MISSING_PARAM);
  }
  return this.request('/api/v1/transaction-with-token-transfer-lookup', {
    chain,
    transaction_hash: transactionHash,
    block_timestamp: blockTimestamp,
  });
}

Also add a // ============= Transaction Endpoints ============= section header to keep the file consistent.


src/commands/research.js — low

Slop comment: // Chains the API rejects without block_timestamp. ...

The inline comment starting on line 349 is genuinely useful for the first sentence (which chains require the timestamp), but the second sentence — "Every other enum value, including 'all', near, and injective, accepts a timestamp-less lookup" — restates what the if-condition already implies (i.e., anything not in the list is not checked). Trimming it to just the rationale would be cleaner:

// bitcoin, tron, ton, starknet, and sui require block_timestamp; all other chains do not.

Token usage: 1,347 input, 4,285 output, 389,394 cache read, 32,937 cache write | Usage Guide

New pushes are reviewed automatically with a 10-minute cooldown between reviews. To request a review at any time, comment @nansen-pr-reviewer re-review.

nansen-pr-reviewer[bot]
nansen-pr-reviewer Bot previously approved these changes Sep 3, 2026

@nansen-pr-reviewer nansen-pr-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Auto-approved

This PR was automatically approved because:

  • Claude recommends approval
  • Claude assessed this as a moderate effort change
  • The effort level is within the auto-approval threshold of 2
  • No high or critical issues were detected
  • Review comment contains non-blocking feedback

If you have any concerns, please request a manual review.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nansen-pr-reviewer[bot]
nansen-pr-reviewer Bot previously approved these changes Sep 3, 2026

@nansen-pr-reviewer nansen-pr-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Auto-approved

This PR was automatically approved because:

  • Claude recommends approval
  • Claude assessed this as a moderate effort change
  • The effort level is within the auto-approval threshold of 2
  • No high or critical issues were detected
  • Review comment contains non-blocking feedback

If you have any concerns, please request a manual review.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nansen-pr-reviewer[bot]
nansen-pr-reviewer Bot previously approved these changes Sep 3, 2026

@nansen-pr-reviewer nansen-pr-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Auto-approved

This PR was automatically approved because:

  • Claude recommends approval
  • Claude assessed this as a moderate effort change
  • The effort level is within the auto-approval threshold of 2
  • No high or critical issues were detected
  • Review comment contains non-blocking feedback

If you have any concerns, please request a manual review.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nansen-pr-reviewer[bot]
nansen-pr-reviewer Bot previously approved these changes Sep 3, 2026

@nansen-pr-reviewer nansen-pr-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Auto-approved

This PR was automatically approved because:

  • Claude recommends approval
  • Claude assessed this as a moderate effort change
  • The effort level is within the auto-approval threshold of 2
  • No high or critical issues were detected
  • Review comment contains non-blocking feedback

If you have any concerns, please request a manual review.

Resolve conflicts with the research subcommands added in #555-#561:
keep every direct subcommand and historical-token-ohlcv in the
subcommand set, help text, README, schema, API client, and tests, and
update the subcommand-count assertion (19 total).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@nansen-pr-reviewer nansen-pr-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Auto-approved

This PR was automatically approved because:

  • Claude recommends approval
  • Claude assessed this as a moderate effort change
  • The effort level is within the auto-approval threshold of 2
  • No high or critical issues were detected
  • Review comment contains non-blocking feedback

If you have any concerns, please request a manual review.

@gulshngill
gulshngill merged commit bb17d97 into main Sep 4, 2026
9 checks passed
@gulshngill
gulshngill deleted the feat/research-transaction-token-transfer-lookup branch September 4, 2026 07:57
@github-actions github-actions Bot mentioned this pull request Sep 4, 2026
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.

1 participant