Skip to content
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.

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

Add Validate impl for ScMap and Map ScVal variant that can be invalid #112

Merged
merged 75 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from 74 commits
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
56c26ee
wip
leighmcculloch Jul 15, 2022
52bdeac
wip
leighmcculloch Jul 15, 2022
dbf46ff
binary
leighmcculloch Jul 15, 2022
29f18c9
additional binary
leighmcculloch Jul 15, 2022
a9d3fcb
wip
leighmcculloch Jul 16, 2022
7e70752
map
leighmcculloch Jul 16, 2022
9dda60c
mapg
leighmcculloch Jul 16, 2022
5758066
fix
leighmcculloch Jul 16, 2022
8efa67f
numbigint
leighmcculloch Jul 16, 2022
d001854
comment
leighmcculloch Jul 16, 2022
58dd168
tryfrom
leighmcculloch Jul 16, 2022
3f56112
fix
leighmcculloch Jul 16, 2022
6e85b33
add comments
leighmcculloch Jul 16, 2022
489014e
comments
leighmcculloch Jul 16, 2022
7cfbbfe
comments
leighmcculloch Jul 16, 2022
d9c09bb
fn
leighmcculloch Jul 16, 2022
5683979
fix
leighmcculloch Jul 16, 2022
e03c50a
dupe
leighmcculloch Jul 16, 2022
569bb50
fn
leighmcculloch Jul 16, 2022
57fae23
static
leighmcculloch Jul 16, 2022
b491f48
fn
leighmcculloch Jul 16, 2022
0d27b30
fn
leighmcculloch Jul 16, 2022
8593a45
wip
leighmcculloch Jul 16, 2022
1f2dbba
fn
leighmcculloch Jul 16, 2022
7605dcc
fn
leighmcculloch Jul 16, 2022
16224f6
fn
leighmcculloch Jul 16, 2022
79c6e32
wip
leighmcculloch Jul 18, 2022
f807c00
wip
leighmcculloch Jul 18, 2022
b15dc93
wip
leighmcculloch Jul 18, 2022
b6f53a2
fix
leighmcculloch Jul 18, 2022
2e16cac
reverse
leighmcculloch Jul 18, 2022
2a33abf
remove
leighmcculloch Jul 18, 2022
9b9ba4b
simplify
leighmcculloch Jul 18, 2022
33e0143
tests
leighmcculloch Jul 18, 2022
1509f98
fix test
leighmcculloch Jul 18, 2022
da3bbdd
fin
leighmcculloch Jul 18, 2022
18c6def
fix
leighmcculloch Jul 18, 2022
a59e850
fix
leighmcculloch Jul 18, 2022
dd31ba7
fix
leighmcculloch Jul 18, 2022
6394429
add big int test
leighmcculloch Jul 18, 2022
20b5eb4
Merge branch 'main' into issue104
leighmcculloch Jul 18, 2022
e94658f
Put base64 functionality behind feature flag
leighmcculloch Jul 18, 2022
c4c6870
wip
leighmcculloch Jul 18, 2022
9a224b3
Merge branch 'main' into base64
leighmcculloch Jul 18, 2022
b3772b6
Merge branch 'base64' into issue106
leighmcculloch Jul 18, 2022
9d91047
add some validations
leighmcculloch Jul 18, 2022
ed0e05c
fix
leighmcculloch Jul 18, 2022
0390aed
basics
leighmcculloch Jul 18, 2022
f070022
tests
leighmcculloch Jul 18, 2022
1f214e1
add mapg
leighmcculloch Jul 19, 2022
32ff6bc
add todo
leighmcculloch Jul 19, 2022
def3584
regen
leighmcculloch Jul 19, 2022
c6c0d9a
Merge branch 'base64' into issue106
leighmcculloch Jul 19, 2022
60b9528
add missing errors doc
leighmcculloch Jul 19, 2022
b2d882f
move
leighmcculloch Jul 19, 2022
447fcad
Merge branch 'issue106' into issue106b
leighmcculloch Jul 19, 2022
0c0aaa4
fix
leighmcculloch Jul 19, 2022
896fc2a
add todo
leighmcculloch Jul 19, 2022
4e4ba8e
remove comment
leighmcculloch Jul 19, 2022
19dfb0b
Merge branch 'issue106' into issue106b
leighmcculloch Jul 19, 2022
dfe469e
wip
leighmcculloch Jul 19, 2022
7f53fc1
Merge branch 'main' into issue106b
leighmcculloch Jul 19, 2022
0ed9633
Add helper for sorted maps, trim comments in validate.
graydon Jul 19, 2022
04e5e18
Merge branch 'main' into issue106b
graydon Jul 20, 2022
3b63c8a
change test name
leighmcculloch Jul 20, 2022
986e1e6
add comment
leighmcculloch Jul 20, 2022
b8090f2
Merge branch 'main' into issue106b
leighmcculloch Jul 20, 2022
0626003
Switch from From to Into
graydon Jul 20, 2022
b2c753c
Merge branch 'main' into issue106b
leighmcculloch Jul 20, 2022
f197cbe
comment
leighmcculloch Jul 20, 2022
529a57a
add error type
leighmcculloch Jul 20, 2022
a1a8d83
make maps work with TryInto impl too for supporting vecs
leighmcculloch Jul 21, 2022
94e8097
add todo
leighmcculloch Jul 21, 2022
4c91f59
error wrangling
leighmcculloch Jul 21, 2022
d16cab4
more general sorted_from
leighmcculloch Jul 21, 2022
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
20 changes: 20 additions & 0 deletions src/curr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ use std::{
io::{BufRead, BufReader, Cursor, Read, Write},
};

/// Error contains all errors returned by functions in this crate. It can be
/// compared via `PartialEq`, however any contained IO errors will only be
/// compared on their `ErrorKind`.
#[derive(Debug)]
pub enum Error {
Invalid,
Expand All @@ -87,6 +90,23 @@ pub enum Error {
Io(io::Error),
}

impl PartialEq for Error {
fn eq(&self, other: &Self) -> bool {
match (self, other) {
(Self::Utf8Error(l), Self::Utf8Error(r)) => l == r,
// IO errors cannot be compared, but in the absence of any more
// meaningful way to compare the errors we compare the kind of error
// and ignore the embedded source error or OS error. The main use
// case for comparing errors outputted by the XDR library is for
// error case testing, and a lack of the ability to compare has a
// detrimental affect on failure testing, so this is a tradeoff.
#[cfg(feature = "std")]
(Self::Io(l), Self::Io(r)) => l.kind() == r.kind(),
_ => core::mem::discriminant(self) == core::mem::discriminant(other),
}
}
}

#[cfg(feature = "std")]
impl error::Error for Error {
#[must_use]
Expand Down
5 changes: 5 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,8 @@ pub use scval_conversions::*;
mod scval_validations;
#[cfg(feature = "next")]
pub use scval_validations::*;

#[cfg(all(feature = "alloc", feature = "next"))]
mod scmap;
#[cfg(all(feature = "alloc", feature = "next"))]
pub use scmap::*;
20 changes: 20 additions & 0 deletions src/next.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ use std::{
io::{BufRead, BufReader, Cursor, Read, Write},
};

/// Error contains all errors returned by functions in this crate. It can be
/// compared via `PartialEq`, however any contained IO errors will only be
/// compared on their `ErrorKind`.
#[derive(Debug)]
pub enum Error {
Invalid,
Expand All @@ -102,6 +105,23 @@ pub enum Error {
Io(io::Error),
}

impl PartialEq for Error {
fn eq(&self, other: &Self) -> bool {
match (self, other) {
(Self::Utf8Error(l), Self::Utf8Error(r)) => l == r,
// IO errors cannot be compared, but in the absence of any more
// meaningful way to compare the errors we compare the kind of error
// and ignore the embedded source error or OS error. The main use
// case for comparing errors outputted by the XDR library is for
// error case testing, and a lack of the ability to compare has a
// detrimental affect on failure testing, so this is a tradeoff.
#[cfg(feature = "std")]
(Self::Io(l), Self::Io(r)) => l.kind() == r.kind(),
_ => core::mem::discriminant(self) == core::mem::discriminant(other),
}
}
}

#[cfg(feature = "std")]
impl error::Error for Error {
#[must_use]
Expand Down
76 changes: 76 additions & 0 deletions src/scmap.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
#![allow(clippy::missing_errors_doc)]

use crate::{Error, ScMap, ScMapEntry, ScVal, Validate};
extern crate alloc;
use alloc::vec::Vec;

impl ScMap {
pub fn sorted_from_entries<I, E>(entries: I) -> Result<ScMap, Error>
where
E: TryInto<ScMapEntry>,
I: Iterator<Item = E>,
{
let mut v = entries
.map(TryInto::try_into)
.collect::<Result<Vec<_>, _>>()
.map_err(|_| Error::Invalid)?;
// TODO: Add tests that prove order consistency of ScVal with RawVal. https://github.com/stellar/rs-stellar-xdr/issues/117
v.sort_by(|a, b| a.key.cmp(&b.key));
leighmcculloch marked this conversation as resolved.
Show resolved Hide resolved
let m = ScMap(v.try_into()?);
// `validate` will further check that there are no duplicates.
m.validate()?;
Ok(m)
}

pub fn sorted_from_pairs<K, V, I>(pairs: I) -> Result<ScMap, Error>
where
K: TryInto<ScVal>,
V: TryInto<ScVal>,
I: Iterator<Item = (K, V)>,
{
Self::sorted_from_entries(pairs)
}
Comment on lines +25 to +32
Copy link
Member Author

Choose a reason for hiding this comment

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

This fn is now technically redundant because the same values can be passed directly to the entries fn. But this fn has a clearer signature that is easier to understand, so I think it is still useful so I kept it.


pub fn sorted_from<K, V, I>(src: I) -> Result<ScMap, Error>
where
K: Into<ScVal>,
V: Into<ScVal>,
I: IntoIterator<Item = (K, V)>,
{
Self::sorted_from_pairs(src.into_iter())
}
}

#[cfg(test)]
mod test {
use super::*;
use alloc::{collections::BTreeMap, vec};

#[test]
fn scmap_from_map() -> Result<(), ()> {
let mut m: BTreeMap<u32, u32> = BTreeMap::new();
m.insert(1, 2);
m.insert(5, 6);
m.insert(3, 4);
let scm = ScMap::sorted_from(m)?;
assert_eq!(scm.0.first().unwrap().key, 1u32.into());
assert_eq!(scm.0.last().unwrap().key, 5u32.into());
Ok(())
}

#[test]
fn scmap_from_pairs() -> Result<(), ()> {
let pairs: Vec<(u32, u32)> = vec![(3, 4), (5, 6), (1, 2)];
let scm = ScMap::sorted_from(pairs)?;
assert_eq!(scm.0.first().unwrap().key, 1u32.into());
assert_eq!(scm.0.last().unwrap().key, 5u32.into());
Ok(())
}

#[test]
fn scmap_from_pairs_containing_duplicate_keys() {
let pairs: Vec<(u32, u32)> = vec![(3, 4), (3, 5), (5, 6), (1, 2)];
let scm = ScMap::sorted_from(pairs);
assert!(scm.is_err());
}
}
20 changes: 19 additions & 1 deletion src/scval_conversions.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{
Hash, PublicKey, ScBigInt, ScHash, ScMap, ScObject, ScStatic, ScStatus, ScSymbol, ScVal, ScVec,
Hash, PublicKey, ScBigInt, ScHash, ScMap, ScMapEntry, ScObject, ScStatic, ScStatus, ScSymbol,
ScVal, ScVec,
};

#[cfg(all(not(feature = "std"), feature = "alloc"))]
Expand All @@ -10,6 +11,8 @@ use alloc::{string::String, vec, vec::Vec};
#[cfg(feature = "num-bigint")]
use num_bigint::{BigInt, Sign};

// TODO: Use the Error type for conversions in this file.

impl From<ScStatic> for ScVal {
fn from(v: ScStatic) -> Self {
Self::Static(v)
Expand Down Expand Up @@ -723,6 +726,21 @@ impl TryFrom<ScVal> for ScMap {
}
}

impl<K, V> TryFrom<(K, V)> for ScMapEntry
where
K: TryInto<ScVal>,
V: TryInto<ScVal>,
{
type Error = ();

fn try_from(v: (K, V)) -> Result<Self, Self::Error> {
Ok(ScMapEntry {
key: v.0.try_into().map_err(|_| ())?,
val: v.1.try_into().map_err(|_| ())?,
})
}
}

// TODO: Add conversions from std::collections::HashMap, im_rcOrdMap, and other
// popular map types to ScMap.

Expand Down
125 changes: 110 additions & 15 deletions src/scval_validations.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
#![allow(clippy::missing_errors_doc)]

use crate::{ScMap, ScObject, ScVal};
use crate::{Error, ScMap, ScObject, ScVal};

pub trait Validate {
type Error;
fn validate(&self) -> Result<(), Self::Error>;
}

impl Validate for ScVal {
type Error = ();
type Error = Error;

fn validate(&self) -> Result<(), Self::Error> {
match self {
Expand All @@ -17,7 +17,7 @@ impl Validate for ScVal {
if *i >= 0 {
Ok(())
} else {
Err(())
Err(Error::Invalid)
}
}

Expand All @@ -28,7 +28,7 @@ impl Validate for ScVal {
{
Ok(())
} else {
Err(())
Err(Error::Invalid)
}
}

Expand All @@ -37,11 +37,11 @@ impl Validate for ScVal {
if b & 0x0fff_ffff_ffff_ffff == *b {
Ok(())
} else {
Err(())
Err(Error::Invalid)
}
}

ScVal::Object(None) => Err(()),
ScVal::Object(None) => Err(Error::Invalid),

ScVal::Object(Some(o)) => match o {
ScObject::Map(m) => m.validate(),
Expand All @@ -63,39 +63,134 @@ impl Validate for ScVal {
}

impl Validate for ScMap {
type Error = ();
type Error = Error;

fn validate(&self) -> Result<(), Self::Error> {
// TODO: Validate that the map is sorted and has no duplicates, or find
// a way to guarantee this to be the case.
todo!()
// Check the map is sorted by key, and there are no keys that are
// duplicates.
if self.windows(2).all(|w| w[0].key < w[1].key) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this .windows function? Can't find its definition.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah cool!

Ok(())
} else {
Err(Error::Invalid)
}
}
}

#[cfg(test)]
mod test {
use crate::{ScVal, Validate};
use crate::{Error, ScVal, Validate};

#[test]
fn u63() {
assert_eq!(ScVal::U63(0).validate(), Ok(()));
assert_eq!(ScVal::U63(1).validate(), Ok(()));
assert_eq!(ScVal::U63(i64::MAX).validate(), Ok(()));
assert_eq!(ScVal::U63(-1).validate(), Err(()));
assert_eq!(ScVal::U63(-1).validate(), Err(Error::Invalid));
}

#[test]
fn symbol() {
assert_eq!(ScVal::Symbol("".try_into().unwrap()).validate(), Ok(()));
assert_eq!(ScVal::Symbol("a0A_".try_into().unwrap()).validate(), Ok(()));
assert_eq!(ScVal::Symbol("]".try_into().unwrap()).validate(), Err(()));
assert_eq!(
ScVal::Symbol("]".try_into().unwrap()).validate(),
Err(Error::Invalid)
);
}

#[test]
fn bitset() {
assert_eq!(ScVal::Bitset(0x0000_0000_0000_0000).validate(), Ok(()));
assert_eq!(ScVal::Bitset(0x0fff_ffff_ffff_ffff).validate(), Ok(()));
assert_eq!(ScVal::Bitset(0x1000_0000_0000_0000).validate(), Err(()));
assert_eq!(ScVal::Bitset(0x1fff_ffff_ffff_ffff).validate(), Err(()));
assert_eq!(
ScVal::Bitset(0x1000_0000_0000_0000).validate(),
Err(Error::Invalid)
);
assert_eq!(
ScVal::Bitset(0x1fff_ffff_ffff_ffff).validate(),
Err(Error::Invalid)
);
}

#[test]
#[cfg(feature = "alloc")]
fn map() {
extern crate alloc;
use crate::{ScMap, ScMapEntry, ScObject};
use alloc::vec;
// Maps should be sorted by key and have no duplicates. The sort order
// is just the "normal" sort order on ScVal emitted by derive(PartialOrd).
assert_eq!(
ScVal::Object(Some(ScObject::Map(ScMap(
vec![
ScMapEntry {
key: ScVal::U63(0),
val: ScVal::U32(1),
},
ScMapEntry {
key: ScVal::U63(1),
val: ScVal::U63(1),
}
]
.try_into()
.unwrap()
))))
.validate(),
Ok(())
);
assert_eq!(
ScVal::Object(Some(ScObject::Map(ScMap(
vec![
ScMapEntry {
key: ScVal::U63(0),
val: ScVal::U63(1),
},
ScMapEntry {
key: ScVal::U63(1),
val: ScVal::U63(1),
}
]
.try_into()
.unwrap()
))))
.validate(),
Ok(())
);
assert_eq!(
ScVal::Object(Some(ScObject::Map(ScMap(
vec![
ScMapEntry {
key: ScVal::U63(2),
val: ScVal::U63(1),
},
ScMapEntry {
key: ScVal::U63(1),
val: ScVal::U63(1),
}
]
.try_into()
.unwrap()
))))
.validate(),
Err(Error::Invalid)
);
assert_eq!(
ScVal::Object(Some(ScObject::Map(ScMap(
vec![
ScMapEntry {
key: ScVal::U32(1),
val: ScVal::U63(1),
},
ScMapEntry {
key: ScVal::U63(2),
val: ScVal::U63(1),
},
]
.try_into()
.unwrap()
))))
.validate(),
Err(Error::Invalid)
);
}
}