Skip to content

Conversation

@tim-barry
Copy link
Contributor

@tim-barry tim-barry commented Dec 4, 2025

When parsing arguments to the basic-transfer.cdc transaction, should use the correct cadence type.

Summary by CodeRabbit

  • Bug Fixes
    • Corrected transaction amount handling to support decimal precision values.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 4, 2025

Walkthrough

A single type assertion correction in the construction service where the transaction amount handling was updated from UInt64 to UFix64, with a corresponding error message adjustment to reflect the new type being used.

Changes

Cohort / File(s) Summary
Type precision adjustment
api/construction_service.go
Changed type assertion in decodeTransferOps from cadence.UInt64 to cadence.UFix64 for transaction amount handling; updated associated error message for consistency

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single file affected with a straightforward type change
  • Minimal scope and homogeneous edit (one type assertion + one error message)
  • No structural or control flow modifications

Poem

🐰 A rabbit's whisper on the breeze,
UFix64 now flows with ease,
Where UInt64 once stood tall,
Precision blooms—fixed point for all! 🌟

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: correcting a Cadence type from UInt64 to UFix64 in a type assertion.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch tim/cadence-use-ufix64

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3818570 and 3093ab5.

📒 Files selected for processing (1)
  • api/construction_service.go (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: franklywatson
Repo: onflow/rosetta PR: 63
File: script/cadence/scripts/get-key-index.cdc:1-4
Timestamp: 2024-10-08T17:04:37.869Z
Learning: User: franklywatson
PR: onflow/rosetta#63
File: script/cadence/scripts/get-key-index.cdc:1-4
Timestamp: 2024-06-27T00:56:13.121Z
Learning: Ensure that all support and suggestions provided for Cadence are specifically tailored to Cadence 1.0, referencing the latest standards and documentation as shared by the user.
Learnt from: franklywatson
Repo: onflow/rosetta PR: 63
File: script/cadence/scripts/get-key-index.cdc:1-4
Timestamp: 2024-06-27T01:02:04.949Z
Learning: User: franklywatson
PR: onflow/rosetta#63
File: script/cadence/scripts/get-key-index.cdc:1-4
Timestamp: 2024-06-27T00:56:13.121Z
Learning: Ensure that all support and suggestions provided for Cadence are specifically tailored to Cadence 1.0, referencing the latest standards and documentation as shared by the user.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: test
🔇 Additional comments (1)
api/construction_service.go (1)

1431-1435: UFix64 decode matches Cadence 1.0 transfer scripts and existing encode path

Switching the amount assertion to cadence.UFix64 is correct and aligns with:

  • The Cadence 1.0 basic-transfer.cdc amount parameter type (UFix64).
  • The encoding side, which already does jsoncdc.Encode(cadence.UFix64(intent.amount)).

uint64(amount) yields the same atomic-unit value you encoded, so downstream Rosetta amounts remain consistent.


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.

@tim-barry tim-barry merged commit 29938cf into main Dec 5, 2025
1 check passed
@tim-barry tim-barry deleted the tim/cadence-use-ufix64 branch December 5, 2025 17:29
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