Skip to content

Commit 5e40d72

Browse files
authored
Merge pull request #92 from silicon-heaven/decimal-improvements
Decimal improvements
2 parents 24bbd0b + eb1dde7 commit 5e40d72

8 files changed

Lines changed: 248 additions & 18 deletions

File tree

.github/workflows/rust.yml

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ permissions:
2020
jobs:
2121
build:
2222
strategy:
23+
fail-fast: false
2324
matrix:
2425
os: [ubuntu-latest, windows-latest]
2526
toolchain: [stable, nightly]
@@ -40,13 +41,18 @@ jobs:
4041
target: ${{ matrix.target }}
4142
toolchain: ${{ matrix.toolchain }}
4243
override: true
44+
components: clippy
4345

4446
- name: Build and Install Binary
4547
run: |
4648
mkdir -p ${{github.workspace}}/install
47-
# FIXME: Remove when this thing makes it into stable
4849
export RUSTFLAGS="-A mismatched_lifetime_syntaxes"
49-
cargo install --path '${{github.workspace}}' --root '${{github.workspace}}/install' --target ${{ matrix.target }} --features cp2cp --locked
50+
FEATURES="cp2cp,serde"
51+
if rustup default | grep nightly; then
52+
FEATURES+=",specialization"
53+
fi
54+
cargo clippy --all-targets --features "${FEATURES}"
55+
cargo install --path '${{github.workspace}}' --root '${{github.workspace}}/install' --target ${{ matrix.target }} --features "${FEATURES}" --locked
5056
shell: bash
5157

5258
- name: Upload Artifact
@@ -93,7 +99,7 @@ jobs:
9399
uses: silicon-heaven/rust-pycobertura-action@v4.0.1
94100
with:
95101
project_name: libshvproto
96-
cargo_test_arguments: --features cp2cp
102+
cargo_test_arguments: --features cp2cp,serde
97103

98104
check-version-bump:
99105
name: Check version bump
@@ -170,7 +176,11 @@ jobs:
170176
- run: |
171177
cargo login <<< "${{secrets.CRATES_IO_TOKEN}}"
172178
if ! PUBLISH_OUTPUT="$(cargo publish --workspace 2>&1)"; then
179+
echo "$PUBLISH_OUTPUT"
173180
if ! [[ "$PUBLISH_OUTPUT" =~ already\ exists\ on\ crates.io\ index ]]; then
174181
exit 1
175182
fi
183+
echo Some crates were already published.
184+
else
185+
echo "$PUBLISH_OUTPUT"
176186
fi

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ name = "shvproto"
55
description = "Rust implementation of the SHV protocol"
66
license = "MIT"
77
repository = "https://github.com/silicon-heaven/libshvproto-rs"
8-
version = "3.6.32"
8+
version = "3.6.33"
99
edition = "2024"
1010

1111
[dependencies]

src/bin/cp2cp.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ fn exit_with_result_and_code(result: &ChainPackRpcBlockResult, error: Option<Rea
5454
let exit_code = if let Some(error) = &error {
5555
match error {
5656
ReadErrorReason::UnexpectedEndOfStream => CODE_NOT_ENOUGH_DATA,
57+
ReadErrorReason::NotEnoughPrecision => CODE_READ_ERROR,
5758
ReadErrorReason::InvalidCharacter => {
5859
eprintln!("Parse input error: {:?}", error);
5960
CODE_READ_ERROR

src/cpon.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,7 @@ mod test
592592
}
593593
test_cpon_round_trip("null", RpcValue::null());
594594
test_cpon_round_trip("false", false);
595+
assert!(RpcValue::from_cpon("faldwa").is_err());
595596
test_cpon_round_trip("true", true);
596597
assert_eq!(RpcValue::from_cpon("0").unwrap().as_i32(), 0);
597598
assert_eq!(RpcValue::from_cpon("123").unwrap().as_i32(), 123);
@@ -602,11 +603,15 @@ mod test
602603
assert_eq!(RpcValue::from_cpon("-0x1000").unwrap().as_i32(), -4096);
603604
test_cpon_round_trip("123.4", Decimal::new(1234, -1));
604605
test_cpon_round_trip("0.123", Decimal::new(123, -3));
605-
assert_eq!(RpcValue::from_cpon("-0.123").unwrap().as_decimal(), Decimal::new(-123, -3));
606+
test_cpon_round_trip("-0.123", Decimal::new(-123, -3));
607+
test_cpon_round_trip("123000000e2", Decimal::new(123000000, 2));
608+
assert_eq!(Decimal::new(10000, 3).to_cpon_string(), "10000000.");
606609
assert_eq!(RpcValue::from_cpon("0e0").unwrap().as_decimal(), Decimal::new(0, 0));
607610
assert_eq!(RpcValue::from_cpon("0.123e3").unwrap().as_decimal(), Decimal::new(123, 0));
608611
test_cpon_round_trip("1000000.", Decimal::new(1000000, 0));
609612
test_cpon_round_trip("50.03138741402532", Decimal::new(5003138741402532, -14));
613+
// We do not support such high precision.
614+
assert!(RpcValue::from_cpon("36.028797018963968").is_err());
610615
assert_eq!(RpcValue::from_cpon(r#""foo""#).unwrap().as_str(), "foo");
611616
assert_eq!(RpcValue::from_cpon(r#""ěščřžýáí""#).unwrap().as_str(), "ěščřžýáí");
612617
assert_eq!(RpcValue::from_cpon("b\"foo\tbar\nbaz\"").unwrap().as_blob(), b"foo\tbar\nbaz");
@@ -709,8 +714,8 @@ mod test
709714
assert_eq!(RpcValue::from_cpon("-0x8000000000000000").unwrap().as_int(), i64::MIN);
710715
assert_eq!(RpcValue::from_cpon("-0x8000000000000001").unwrap().as_int(), i64::MIN);
711716

712-
assert_eq!(RpcValue::from_cpon("1.23456789012345678901234567890123456789012345678901234567890").unwrap().as_decimal(), Decimal::new(1234567890123456789, -18));
713-
assert_eq!(RpcValue::from_cpon("12345678901234567890123456789012345678901234567890123456.7890").unwrap().as_decimal(), Decimal::new(i64::MAX, 0));
714-
assert_eq!(RpcValue::from_cpon("123456789012345678901234567890123456789012345678901234567890.").unwrap().as_decimal(), Decimal::new(i64::MAX, 0));
717+
assert!(RpcValue::from_cpon("1.23456789012345678901234567890123456789012345678901234567890").is_err());
718+
assert!(RpcValue::from_cpon("12345678901234567890123456789012345678901234567890123456.7890").is_err());
719+
assert!(RpcValue::from_cpon("123456789012345678901234567890123456789012345678901234567890.").is_err());
715720
}
716721
}

src/decimal.rs

Lines changed: 214 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
1-
//use crate::rpcvalue::RpcValue;
2-
31
/// mantisa: 56, exponent: 8;
42
/// I'm storing whole Decimal in one i64 to keep size_of RpcValue == 24
5-
#[derive(Debug, Copy, Clone, PartialEq)]
3+
#[derive(Debug, Copy, Clone)]
64
pub struct Decimal(i64);
75

86
impl Decimal {
@@ -12,6 +10,20 @@ impl Decimal {
1210
n |= (exponent as i64) & 0xff;
1311
Decimal(n)
1412
}
13+
pub fn normalize(&self) -> Decimal {
14+
let (mut mantissa, mut exponent) = self.decode();
15+
if mantissa == 0 {
16+
return Decimal::new(0, 0);
17+
}
18+
19+
20+
while mantissa != 0 && mantissa % 10 == 0 {
21+
mantissa /= 10;
22+
exponent += 1;
23+
}
24+
25+
Decimal::new(mantissa, exponent)
26+
}
1527
pub fn decode(&self) -> (i64, i8) {
1628
let m = self.0 >> 8;
1729
let e = self.0 as i8;
@@ -30,7 +42,6 @@ impl Decimal {
3042
mantissa = -mantissa;
3143
neg = true;
3244
}
33-
//let buff: Vec<u8> = Vec::new();
3445
let mut s = mantissa.to_string();
3546

3647
let n = s.len() as i8;
@@ -87,3 +98,202 @@ impl Decimal {
8798
}
8899

89100

101+
use std::cmp::Ordering;
102+
103+
impl Ord for Decimal {
104+
fn cmp(&self, other: &Self) -> Ordering {
105+
let mut a = self.normalize();
106+
let mut b = other.normalize();
107+
108+
if a.exponent() == b.exponent() {
109+
return a.mantissa().cmp(&b.mantissa());
110+
}
111+
112+
let (to_scale, not_to_scale) = if a.exponent() > b.exponent() {
113+
(&mut a, &mut b)
114+
} else {
115+
(&mut b, &mut a)
116+
};
117+
let exponent_diff = (not_to_scale.exponent() - to_scale.exponent()).abs();
118+
if let Some(scaled) = 10_i64.checked_pow(exponent_diff as u32).and_then(|multiply_by| to_scale.mantissa().checked_mul(multiply_by)) {
119+
*to_scale = Decimal::new(scaled, to_scale.exponent());
120+
return a.mantissa().cmp(&b.mantissa());
121+
}
122+
123+
let da = a.to_f64();
124+
let db = b.to_f64();
125+
126+
if da < db {
127+
return Ordering::Less;
128+
}
129+
if da > db {
130+
return Ordering::Greater;
131+
}
132+
133+
// We can't actually be sure whether the Decimals are equal here, because double does not have strong ordering.
134+
Ordering::Equal
135+
}
136+
}
137+
138+
impl PartialOrd for Decimal {
139+
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
140+
Some(self.cmp(other))
141+
}
142+
}
143+
144+
impl PartialEq for Decimal {
145+
fn eq(&self, other: &Self) -> bool {
146+
self.cmp(other) == Ordering::Equal
147+
}
148+
}
149+
150+
impl Eq for Decimal {}
151+
#[cfg(test)]
152+
mod tests {
153+
use super::*;
154+
155+
#[test]
156+
fn decimal_normalization_removes_trailing_zeros() {
157+
let d1 = Decimal::new(1000, -3);
158+
let n1 = Decimal::normalize(&d1);
159+
assert_eq!(n1.mantissa(), 1);
160+
assert_eq!(n1.exponent(), 0);
161+
162+
let d2 = Decimal::new(1200, -3);
163+
let n2 = Decimal::normalize(&d2);
164+
assert_eq!(n2.mantissa(), 12);
165+
assert_eq!(n2.exponent(), -1);
166+
167+
let d3 = Decimal::new(500, -1);
168+
let n3 = Decimal::normalize(&d3);
169+
assert_eq!(n3.mantissa(), 5);
170+
assert_eq!(n3.exponent(), 1);
171+
}
172+
173+
#[test]
174+
fn decimal_normalization_zero() {
175+
let zero = Decimal::new(0, -10);
176+
let n = Decimal::normalize(&zero);
177+
assert_eq!(n.mantissa(), 0);
178+
assert_eq!(n.exponent(), 0);
179+
}
180+
181+
#[test]
182+
fn decimal_normalization_negative() {
183+
let d = Decimal::new(-5000, -3);
184+
let n = Decimal::normalize(&d);
185+
assert_eq!(n.mantissa(), -5);
186+
assert_eq!(n.exponent(), 0);
187+
}
188+
189+
#[test]
190+
fn decimal_normalization_preserves_value() {
191+
// This also tests conversion to f64 with a positive/negative/zero exponent.
192+
for exp in [-3, 0, 3] {
193+
let d = Decimal::new(1200, exp);
194+
let n = Decimal::normalize(&d);
195+
let diff = (d.to_f64() - n.to_f64()).abs();
196+
assert!(diff < 1e-12);
197+
}
198+
}
199+
200+
#[test]
201+
fn decimal_equality_and_normalization() {
202+
let d1 = Decimal::new(100, -2);
203+
let d2 = Decimal::new(1, 0);
204+
let d3 = Decimal::new(10, -1);
205+
206+
assert_eq!(d1, d2);
207+
assert_eq!(d2, d3);
208+
assert!(d1 <= d3);
209+
assert!(d1 >= d3);
210+
}
211+
212+
#[test]
213+
fn decimal_less_and_greater() {
214+
let d1 = Decimal::new(100, -2);
215+
let smaller = Decimal::new(5, -1);
216+
let larger = Decimal::new(2, 0);
217+
218+
assert!(smaller < d1);
219+
assert!(d1 > smaller);
220+
assert!(d1 < larger);
221+
assert!(larger > d1);
222+
assert!(d1 <= larger);
223+
assert!(larger >= d1);
224+
}
225+
226+
#[test]
227+
fn decimal_negative_numbers() {
228+
let neg1 = Decimal::new(-5, 0);
229+
let neg2 = Decimal::new(-50, -1);
230+
let pos = Decimal::new(5, 0);
231+
232+
assert_eq!(neg1, neg2);
233+
assert!(neg1 < pos);
234+
assert!(pos > neg2);
235+
}
236+
237+
#[test]
238+
fn decimal_zero_edge_cases() {
239+
let zero1 = Decimal::new(0, 0);
240+
let zero2 = Decimal::new(0, 5);
241+
242+
assert_eq!(zero1, zero2);
243+
assert!((zero1 >= zero2));
244+
assert!((zero1 <= zero2));
245+
assert!(zero1 <= zero2);
246+
assert!(zero1 >= zero2);
247+
}
248+
249+
#[test]
250+
fn decimal_large_exponents() {
251+
let big1 = Decimal::new(1, 3);
252+
let big2 = Decimal::new(1000, 0);
253+
let big3 = Decimal::new(1, 4);
254+
255+
assert_eq!(big1, big2);
256+
assert!(big1 < big3);
257+
assert!(big3 > big2);
258+
}
259+
260+
#[test]
261+
fn decimal_comparison_fallback_to_double_due_to_pow10_overflow() {
262+
// exponent difference so large that pow10(diff) must fail
263+
let a = Decimal::new(1, 0);
264+
let b = Decimal::new(1, 127);
265+
266+
// numeric values: a = 1, b = 1e400
267+
assert!(a < b);
268+
assert!(b > a);
269+
}
270+
271+
#[test]
272+
fn decimal_comparison_fallback_to_double_due_to_mantissa_overflow() {
273+
// pow10(diff) fits, but mantissa * pow10(diff) overflows i64
274+
let a = Decimal::new(i64::MAX / 18, 6);
275+
let b = Decimal::new(1, 1);
276+
277+
// scaling `a` by 10 would overflow, forcing double fallback
278+
assert!(a > b);
279+
assert!(b < a);
280+
}
281+
282+
#[test]
283+
fn decimal_comparison_fallback_negative_values() {
284+
let a = Decimal::new(-1, 127);
285+
let b = Decimal::new(-1, 126);
286+
287+
assert!(a < b);
288+
assert!(b > a);
289+
}
290+
291+
#[test]
292+
fn decimal_comparison_fallback_zero_vs_huge() {
293+
let zero = Decimal::new(0, 0);
294+
let huge = Decimal::new(1, 127);
295+
296+
assert!(zero < huge);
297+
assert!(huge > zero);
298+
}
299+
}

src/json.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -567,8 +567,8 @@ mod test
567567
assert_eq!(RpcValue::from_json("-0x8000000000000000").unwrap().as_int(), i64::MIN);
568568
assert_eq!(RpcValue::from_json("-0x8000000000000001").unwrap().as_int(), i64::MIN);
569569

570-
assert_eq!(RpcValue::from_json("1.23456789012345678901234567890123456789012345678901234567890").unwrap().as_decimal(), Decimal::new(1234567890123456789, -18));
571-
assert_eq!(RpcValue::from_json("12345678901234567890123456789012345678901234567890123456.7890").unwrap().as_decimal(), Decimal::new(i64::MAX, 0));
572-
assert_eq!(RpcValue::from_json("123456789012345678901234567890123456789012345678901234567890.").unwrap().as_decimal(), Decimal::new(i64::MAX, 0));
570+
assert!(RpcValue::from_json("1.23456789012345678901234567890123456789012345678901234567890").is_err());
571+
assert!(RpcValue::from_json("12345678901234567890123456789012345678901234567890123456.7890").is_err());
572+
assert!(RpcValue::from_json("123456789012345678901234567890123456789012345678901234567890.").is_err());
573573
}
574574
}

src/reader.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::rpcvalue::Value;
66
pub enum ReadErrorReason {
77
UnexpectedEndOfStream,
88
InvalidCharacter,
9+
NotEnoughPrecision,
910
}
1011
#[derive(Debug)]
1112
pub struct ReadError {

src/textrdwr.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ pub trait TextReader : Reader {
9494
for c in token.as_bytes() {
9595
let b = self.get_byte()?;
9696
if b != *c {
97-
return Err(self.make_error(&format!("Incomplete '{}' literal.", token), ReadErrorReason::InvalidCharacter))
97+
return Err(self.make_error(&format!("Expected '{}'.", token), ReadErrorReason::InvalidCharacter))
9898
}
9999
}
100100
Ok(())
@@ -234,6 +234,9 @@ pub trait TextReader : Reader {
234234
let ReadInt { value, digit_cnt, is_overflow, .. } = self.read_int(mantissa, true)?;
235235
decimal_overflow = decimal_overflow || is_overflow;
236236
mantissa = value;
237+
if mantissa >= 36028797018963968 {
238+
decimal_overflow = true;
239+
}
237240
dec_cnt = digit_cnt as i64;
238241
}
239242
b'e' | b'E' => {
@@ -257,8 +260,8 @@ pub trait TextReader : Reader {
257260
}
258261
let mantissa = if is_negative { -mantissa } else { mantissa };
259262
if is_decimal {
260-
if decimal_overflow && dec_cnt == 0 {
261-
return Ok(Value::from(Decimal::new(if is_negative {i64::MIN} else {i64::MAX}, 0)))
263+
if decimal_overflow {
264+
return Err(self.make_error("Not enough precision to read the Decimal", ReadErrorReason::InvalidCharacter))
262265
}
263266
return Ok(Value::from(Decimal::new(mantissa, (exponent - dec_cnt) as i8)))
264267
}

0 commit comments

Comments
 (0)