Skip to content
This repository was archived by the owner on Mar 23, 2021. It is now read-only.

Commit 6455f6e

Browse files
thomaseizingerluckysori
authored andcommitted
Refactor deserialization and serialization of ledger and asset values
We make heavy use of serde's "from" and "into" features to explicitly model the format we expect on the HTTP API and the format we send out. This allows us to shift most of the error handling to serde, which in turn gives us better error messages. They are not perfect but about as good as it gets if we stick to deserializing JSON with serde. We could potentially have better error messages by: - either taking apart the JSON manually but that would be a huge coding effort and cumbersome to maintain because we could not make use of many features of serde. - simplifying the JSON model to reduce the complexity in the deserialization process.
1 parent 39d0e05 commit 6455f6e

File tree

15 files changed

+670
-721
lines changed

15 files changed

+670
-721
lines changed

api_tests/dry/peers_using_ip.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,8 @@ import { sleep } from "../lib/util";
5454
},
5555
});
5656

57-
expect(res.error).to.be.false;
58-
expect(res.status).to.equal(201);
59-
expect(res.header.location).to.be.a("string");
57+
expect(res).to.have.status(201);
58+
expect(res).to.have.header("location");
6059
});
6160

6261
it("[Alice] Should not see any peers because the address did not resolve to the given PeerID", async () => {

api_tests/dry/sanity.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ setTimeout(async function() {
6363
"content-type",
6464
"application/problem+json"
6565
);
66-
expect(res.body.title).to.equal("Swap not supported.");
66+
expect(res.body.title).to.equal("Invalid body.");
6767
});
6868

6969
it("[Alice] Returns 400 bad request for malformed requests", async () => {
@@ -78,7 +78,7 @@ setTimeout(async function() {
7878
"content-type",
7979
"application/problem+json"
8080
);
81-
expect(res.body.title).to.equal("Bad Request");
81+
expect(res.body.title).to.equal("Invalid body.");
8282
});
8383

8484
it("[Alice] Should have no peers before making a swap request", async () => {

cnd/src/http_api/action.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use anyhow::Context;
1515
use blockchain_contracts::bitcoin::witness;
1616
use http_api_problem::HttpApiProblem;
1717
use serde::{Deserialize, Serialize};
18-
use std::convert::Infallible;
18+
use std::convert::{Infallible, TryInto};
1919
use warp::http::StatusCode;
2020

2121
pub trait ToSirenAction {
@@ -246,7 +246,7 @@ impl IntoResponsePayload for ethereum::DeployContract {
246246
amount,
247247
gas_limit,
248248
chain_id,
249-
network: chain_id.into(),
249+
network: chain_id.try_into()?,
250250
}),
251251
_ => Err(anyhow::Error::from(UnexpectedQueryParameters {
252252
action: "ethereum::ContractDeploy",
@@ -280,7 +280,7 @@ impl IntoResponsePayload for ethereum::CallContract {
280280
data,
281281
gas_limit,
282282
chain_id,
283-
network: chain_id.into(),
283+
network: chain_id.try_into()?,
284284
min_block_timestamp,
285285
}),
286286
_ => Err(anyhow::Error::from(UnexpectedQueryParameters {
@@ -353,7 +353,7 @@ mod test {
353353
data: None,
354354
gas_limit: U256::from(1),
355355
chain_id,
356-
network: chain_id.into(),
356+
network: chain_id.try_into().unwrap(),
357357
min_block_timestamp: None,
358358
};
359359
let serialized = serde_json::to_string(&contract).unwrap();

cnd/src/http_api/asset.rs

Lines changed: 0 additions & 129 deletions
This file was deleted.

cnd/src/http_api/ethereum_network.rs

Lines changed: 24 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -22,43 +22,46 @@ pub enum Network {
2222
Regtest,
2323
#[strum(serialize = "ropsten")]
2424
Ropsten,
25-
#[strum(serialize = "unknown")]
26-
Unknown,
2725
}
2826

27+
#[derive(Debug, thiserror::Error)]
28+
#[error("chain with id {0} is unknown")]
29+
pub struct UnknownChainId(String);
30+
2931
impl Network {
30-
pub fn from_network_id(s: String) -> Self {
31-
match s.as_str() {
32+
pub fn from_network_id(s: &str) -> Result<Self, UnknownChainId> {
33+
Ok(match s {
3234
"1" => Network::Mainnet,
3335
"3" => Network::Ropsten,
3436
"17" => Network::Regtest,
35-
_ => Network::Unknown,
36-
}
37+
_ => return Err(UnknownChainId(s.to_string())),
38+
})
3739
}
3840
}
3941

40-
impl From<ChainId> for Network {
41-
fn from(chain: ChainId) -> Self {
42-
Network::from_network_id(u32::from(chain).to_string())
42+
impl TryFrom<ChainId> for Network {
43+
type Error = UnknownChainId;
44+
45+
fn try_from(value: ChainId) -> Result<Self, Self::Error> {
46+
let value = u32::from(value).to_string();
47+
Network::from_network_id(value.as_str())
4348
}
4449
}
4550

46-
impl TryFrom<Network> for ChainId {
47-
type Error = ();
48-
49-
fn try_from(network: Network) -> Result<Self, ()> {
51+
impl From<Network> for ChainId {
52+
fn from(network: Network) -> Self {
5053
match network {
51-
Network::Mainnet => Ok(ChainId::mainnet()),
52-
Network::Regtest => Ok(ChainId::regtest()),
53-
Network::Ropsten => Ok(ChainId::ropsten()),
54-
Network::Unknown => Err(()),
54+
Network::Mainnet => ChainId::mainnet(),
55+
Network::Regtest => ChainId::regtest(),
56+
Network::Ropsten => ChainId::ropsten(),
5557
}
5658
}
5759
}
5860

5961
#[cfg(test)]
6062
mod test {
6163
use super::*;
64+
use spectral::prelude::*;
6265
use std::fmt::Display;
6366

6467
#[test]
@@ -74,22 +77,10 @@ mod test {
7477

7578
#[test]
7679
fn from_version() {
77-
assert_eq!(
78-
Network::from_network_id(String::from("1")),
79-
Network::Mainnet
80-
);
81-
assert_eq!(
82-
Network::from_network_id(String::from("3")),
83-
Network::Ropsten
84-
);
85-
assert_eq!(
86-
Network::from_network_id(String::from("17")),
87-
Network::Regtest
88-
);
89-
assert_eq!(
90-
Network::from_network_id(String::from("-1")),
91-
Network::Unknown
92-
);
80+
assert_that(&Network::from_network_id("1")).is_ok_containing(Network::Mainnet);
81+
assert_that(&Network::from_network_id("3")).is_ok_containing(Network::Ropsten);
82+
assert_that(&Network::from_network_id("17")).is_ok_containing(Network::Regtest);
83+
assert_that(&Network::from_network_id("-1")).is_err();
9384
}
9485

9586
fn assert_display<T: Display>(_t: T) {}

cnd/src/http_api/impl_serialize_http.rs

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,35 +3,6 @@ macro_rules! _count {
33
($x:tt $($xs:tt)*) => (1usize + _count!($($xs)*));
44
}
55

6-
macro_rules! impl_serialize_type_name_with_fields {
7-
($type:ty $(:= $name:tt)? { $($field_name:tt $(=> $field_value:ident)?),* }) => {
8-
impl Serialize for Http<$type> {
9-
10-
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
11-
where
12-
S: Serializer,
13-
{
14-
let params = _count!($($name)*);
15-
let mut state = serializer.serialize_struct("", 1 + params)?;
16-
17-
#[allow(unused_variables)]
18-
let name = stringify!($type).to_lowercase();
19-
#[allow(unused_variables)]
20-
let name = name.as_str();
21-
$(let name = $name;)?
22-
23-
state.serialize_field("name", name)?;
24-
25-
$(
26-
state.serialize_field($field_name, &(self.0)$(.$field_value)?)?;
27-
)*
28-
29-
state.end()
30-
}
31-
}
32-
};
33-
}
34-
356
macro_rules! impl_serialize_type_with_fields {
367
($type:ty { $($field_name:tt $(=> $field_value:ident)?),* }) => {
378
impl Serialize for Http<$type> {

0 commit comments

Comments
 (0)