Skip to content

fix: reject negative TAO amounts in staking and transfer params #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sunnysmol wants to merge 2 commits into
base: main
Choose a base branch
from
+77 −13
Open

fix: reject negative TAO amounts in staking and transfer params #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4340ce9
fix: reject negative TAO amounts in staking and transfer params
Mar 9, 2026
6e5d077
fix: also reject NaN and Infinity TAO amounts
Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion bittensor-rs/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ pub enum BittensorError {

#[error("Insufficient balance: {available} < {required}")]
InsufficientBalance { available: u64, required: u64 },

#[error("Invalid amount: {reason}")]
InvalidAmount { reason: String },
}

/// Classification of errors for retry logic
Expand Down Expand Up @@ -443,7 +446,8 @@ impl BittensorError {
| BittensorError::MaxRetriesExceeded { .. }
| BittensorError::BackoffTimeoutReached { .. }
| BittensorError::BlockNotFound { .. }
| BittensorError::InvalidBlockNumber { .. } => ErrorCategory::Permanent,
| BittensorError::InvalidBlockNumber { .. }
| BittensorError::InvalidAmount { .. } => ErrorCategory::Permanent,

// Legacy errors - categorize based on content
BittensorError::RpcError { message }
Expand Down
42 changes: 36 additions & 6 deletions bittensor-rs/src/extrinsics/staking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,20 @@ impl StakeParams {
/// "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY",
/// 1, // netuid
/// 1.5 // TAO
/// );
/// ).unwrap();
/// assert_eq!(params.amount_rao, 1_500_000_000);
/// ```
pub fn new_tao(hotkey: &str, netuid: u16, amount_tao: f64) -> Self {
Self {
pub fn new_tao(hotkey: &str, netuid: u16, amount_tao: f64) -> Result<Self, BittensorError> {
if !amount_tao.is_finite() || amount_tao < 0.0 {
return Err(BittensorError::InvalidAmount {
reason: format!("TAO amount must be a non-negative finite number, got {}", amount_tao),
});
}
Ok(Self {
hotkey: hotkey.to_string(),
netuid,
amount_rao: (amount_tao * 1_000_000_000.0) as u64,
}
})
Comment on lines +44 to +54
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate and read the staking.rs file to verify the implementation
find . -name "staking.rs" -type f | head -5

Repository: one-covenant/bittensor-rs

Length of output: 109

🏁 Script executed:

# Read the constructor implementation and surrounding code
cat -n bittensor-rs/src/extrinsics/staking.rs | sed -n '40,60p'

Repository: one-covenant/bittensor-rs

Length of output: 1004

🏁 Script executed:

# Check for any bounds validation elsewhere in the file
rg "18\.446|u64::MAX|overflow" bittensor-rs/src/extrinsics/staking.rs

Repository: one-covenant/bittensor-rs

Length of output: 51

🏁 Script executed:

# Look for test cases related to StakeParams
rg -A 10 "fn.*test.*new_tao|#\[test\]" bittensor-rs/src/extrinsics/staking.rs | head -100

Repository: one-covenant/bittensor-rs

Length of output: 1243

🏁 Script executed:

# Search for any validation of upper bounds (e.g., max TAO value)
rg "amount_tao.*<|>.*amount_tao|max.*tao" bittensor-rs/src/extrinsics/staking.rs -i

Repository: one-covenant/bittensor-rs

Length of output: 222

Add upper-bound validation before casting amount_tao to prevent silent overflow.

The current validation checks is_finite() and >= 0.0, but this does not prevent values from overflowing when scaled by 1,000,000,000. For example, any amount_tao > ~18.4 billion will exceed u64::MAX after scaling, causing silent truncation or wraparound in the as u64 cast. Add a check to reject values above the representable ceiling (approximately 18.446 TAO) and add a regression test for oversized positive inputs.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bittensor-rs/src/extrinsics/staking.rs` around lines 44 - 54, In new_tao, add
an upper-bound validation before casting amount_tao to amount_rao to prevent
overflow: compute the maximum representable TAO as (u64::MAX as f64) /
1_000_000_000.0 and if amount_tao > max_tao return
Err(BittensorError::InvalidAmount { reason: format!(...) }); then only perform
the multiplication and as u64 cast when amount_tao is within [0.0, max_tao];
update the error message to mention the exceedance and the max allowed value and
add a regression test that calls new_tao with an oversized positive input (e.g.
> max_tao) asserting it returns the InvalidAmount error.

}

/// Create new stake params with amount in RAO
Expand Down Expand Up @@ -83,7 +88,7 @@ impl StakeParams {
/// "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY",
/// 1, // netuid
/// 1.0 // TAO amount
/// );
/// )?;
/// let result = add_stake(client, signer, params).await?;
/// Ok(())
/// }
Expand Down Expand Up @@ -222,11 +227,36 @@ mod tests {
#[test]
fn test_stake_params_tao() {
let params =
StakeParams::new_tao("5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY", 1, 1.5);
StakeParams::new_tao("5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY", 1, 1.5)
.unwrap();
assert_eq!(params.amount_rao, 1_500_000_000);
assert_eq!(params.netuid, 1);
}

#[test]
fn test_stake_params_negative_tao() {
let result =
StakeParams::new_tao("5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY", 1, -1.0);
assert!(result.is_err());
}

#[test]
fn test_stake_params_nan_tao() {
let result =
StakeParams::new_tao("5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY", 1, f64::NAN);
assert!(result.is_err());
}

#[test]
fn test_stake_params_infinity_tao() {
let result = StakeParams::new_tao(
"5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY",
1,
f64::INFINITY,
);
assert!(result.is_err());
}

#[test]
fn test_stake_params_rao() {
let params =
Expand Down
42 changes: 36 additions & 6 deletions bittensor-rs/src/extrinsics/transfer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,20 @@ impl TransferParams {
/// let params = TransferParams::new_tao(
/// "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY",
/// 1.5
/// );
/// ).unwrap();
/// assert_eq!(params.amount_rao, 1_500_000_000);
/// ```
pub fn new_tao(dest: &str, amount_tao: f64) -> Self {
Self {
pub fn new_tao(dest: &str, amount_tao: f64) -> Result<Self, BittensorError> {
if !amount_tao.is_finite() || amount_tao < 0.0 {
return Err(BittensorError::InvalidAmount {
reason: format!("TAO amount must be a non-negative finite number, got {}", amount_tao),
});
}
Ok(Self {
dest: dest.to_string(),
amount_rao: (amount_tao * 1_000_000_000.0) as u64,
keep_alive: true,
}
})
Comment on lines +42 to +52
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Validate the scaled RAO range as well, not just the raw TAO input.

The current guard still accepts large finite TAO amounts whose scaled RAO value no longer fits in u64. That means new_tao() can still silently build an invalid amount_rao via the as u64 cast instead of failing fast with InvalidAmount. Please add an explicit upper-bound check and a regression test for an oversized finite amount.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bittensor-rs/src/extrinsics/transfer.rs` around lines 42 - 52, new_tao
currently only validates the TAO f64 input but then scales and casts to u64
which can overflow silently; update new_tao to compute the scaled value
(amount_tao * 1_000_000_000.0) and verify it is finite and <= u64::MAX (and >=
0) before casting, returning BittensorError::InvalidAmount if out of range, and
assign amount_rao only after that safe check; also add a regression test that
calls new_tao with a large finite TAO that would exceed u64 when scaled and
asserts it returns InvalidAmount.

}

/// Create new transfer params with amount in RAO
Expand Down Expand Up @@ -84,7 +89,7 @@ impl TransferParams {
/// let params = TransferParams::new_tao(
/// "5FHneW46xGXgs5mUiveU4sbTyGBzmstUspZC92UhjJM694ty",
/// 1.0
/// );
/// )?;
/// let result = transfer(client, signer, params).await?;
/// Ok(())
/// }
Expand Down Expand Up @@ -218,11 +223,35 @@ mod tests {
#[test]
fn test_transfer_params_tao() {
let params =
TransferParams::new_tao("5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY", 1.5);
TransferParams::new_tao("5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY", 1.5)
.unwrap();
assert_eq!(params.amount_rao, 1_500_000_000);
assert!(params.keep_alive);
}

#[test]
fn test_transfer_params_negative_tao() {
let result =
TransferParams::new_tao("5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY", -1.0);
assert!(result.is_err());
}

#[test]
fn test_transfer_params_nan_tao() {
let result =
TransferParams::new_tao("5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY", f64::NAN);
assert!(result.is_err());
}

#[test]
fn test_transfer_params_infinity_tao() {
let result = TransferParams::new_tao(
"5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY",
f64::INFINITY,
);
assert!(result.is_err());
}

#[test]
fn test_transfer_params_rao() {
let params =
Expand All @@ -234,6 +263,7 @@ mod tests {
fn test_transfer_params_builder() {
let params =
TransferParams::new_tao("5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY", 1.0)
.unwrap()
.keep_alive(false);

assert!(!params.keep_alive);
Expand Down