search

opentensor / subtensor

Bittensor Blockchain Layer
The Unlicense
145 stars 149 forks source link

Ban unsafe arithmetic operations #485

Closed keithtensor closed 3 months ago

keithtensor commented 4 months ago

Devnet Companion: #529

This PR bans raw arithmetic operations such as those with the + - / * symbols, and instead replaces them with either the saturating equivalent, or the checked equivalent.

Resolves #303.

sam0x17 commented 4 months ago

looks like some conflicts + CI failures now @keithtensor

sam0x17 commented 3 months ago

So yeah again, should have a companion that merges this branch into devnet-ready. Not sure what this opentensor:development is or why people keep opening PRs into it 😕

sam0x17 commented 3 months ago

but yeah if this is already tested on devnet, should add the devnet-pass label, if this is already tested on testnet, should add the testnet-pass label, at which point it should be able to merge into main

Mananitade commented 3 months ago

I'm curious about the outright ban of the operations + - / *, since these are rather common operations. Have you considered an alternative of catching overflow exceptions? To me, it seems like an overkill, but I'm open to being convinced otherwise. Is it considered good practice in the Rust community to ban these operations? What are the tradeoffs (how much slower does it make things run)?

sam0x17 commented 3 months ago

I'm curious about the outright ban of the operations + - / *, since these are rather common operations. Have you considered an alternative of catching overflow exceptions? To me, it seems like an overkill, but I'm open to being convinced otherwise. Is it considered good practice in the Rust community to ban these operations? What are the tradeoffs (how much slower does it make things run)?

main reason being panicking in an extrinsic is enough to brick a substrate-based chain. On parity's end they have always said you should only use checked math in extrinsics.