Skip to content
This repository was archived by the owner on Mar 23, 2021. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 5 additions & 8 deletions cnd/src/btsieve/ethereum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,13 @@ use crate::{
btsieve::{
find_relevant_blocks, BlockByHash, BlockHash, LatestBlock, Predates, PreviousBlockHash,
},
ethereum::{Address, Bytes, Input, Log, Transaction, TransactionReceipt, H256, U256},
ethereum::{Address, Block, Bytes, Hash, Input, Log, Transaction, TransactionReceipt, U256},
};
use anyhow;
use async_trait::async_trait;
use chrono::NaiveDateTime;
use genawaiter::{sync::Gen, GeneratorState};

type Hash = H256;
type Block = crate::ethereum::Block;

#[async_trait]
pub trait ReceiptByHash: Send + Sync + 'static {
async fn receipt_by_hash(&self, transaction_hash: Hash) -> anyhow::Result<TransactionReceipt>;
Expand All @@ -24,7 +21,7 @@ pub trait ReceiptByHash: Send + Sync + 'static {
impl BlockHash for Block {
type BlockHash = Hash;

fn block_hash(&self) -> H256 {
fn block_hash(&self) -> Hash {
self.hash
.expect("Connector returned latest block with null hash")
}
Expand All @@ -33,7 +30,7 @@ impl BlockHash for Block {
impl PreviousBlockHash for Block {
type BlockHash = Hash;

fn previous_block_hash(&self) -> H256 {
fn previous_block_hash(&self) -> Hash {
self.parent_hash
}
}
Expand Down Expand Up @@ -174,7 +171,7 @@ where
topic.as_ref().map_or(true, |topic| {
block
.logs_bloom
.contains_input(Input::Raw(topic.0.as_ref()))
.contains_input(Input::Raw(&topic.0.as_bytes()))
})
});
if !maybe_contains_transaction {
Expand Down Expand Up @@ -227,7 +224,7 @@ impl Predates for Block {

#[derive(Clone, Copy, Default, Eq, PartialEq, serde::Serialize, serdebug::SerDebug)]
#[serde(transparent)]
pub struct Topic(pub H256);
pub struct Topic(pub Hash);

/// Event works similar to web3 filters:
/// https://web3js.readthedocs.io/en/1.0/web3-eth-subscribe.html?highlight=filter#subscribe-logs
Expand Down
6 changes: 3 additions & 3 deletions cnd/src/btsieve/ethereum/web3_connector.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
btsieve::{ethereum::ReceiptByHash, BlockByHash, LatestBlock},
config::validation::FetchNetworkId,
ethereum::{TransactionReceipt, H256},
ethereum::{Hash, TransactionReceipt},
jsonrpc,
swap_protocols::ledger::ethereum::ChainId,
};
Expand Down Expand Up @@ -45,7 +45,7 @@ impl LatestBlock for Web3Connector {
#[async_trait]
impl BlockByHash for Web3Connector {
type Block = crate::ethereum::Block;
type BlockHash = crate::ethereum::H256;
type BlockHash = crate::ethereum::Hash;

async fn block_by_hash(&self, block_hash: Self::BlockHash) -> anyhow::Result<Self::Block> {
let block = self
Expand All @@ -64,7 +64,7 @@ impl BlockByHash for Web3Connector {

#[async_trait]
impl ReceiptByHash for Web3Connector {
async fn receipt_by_hash(&self, transaction_hash: H256) -> anyhow::Result<TransactionReceipt> {
async fn receipt_by_hash(&self, transaction_hash: Hash) -> anyhow::Result<TransactionReceipt> {
let receipt = self
.client
.send(jsonrpc::Request::new("eth_getTransactionReceipt", vec![
Expand Down
2 changes: 1 addition & 1 deletion cnd/src/comit_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ mod tests {
#[test]
fn erc20_quantity_to_header() -> Result<(), serde_json::Error> {
let quantity = asset::Erc20::new(
Address::zero(),
Address::from([0u8; 20]),
asset::Erc20Quantity::from_wei(U256::from(100_000_000_000_000u64)),
);
let header = AssetKind::from(quantity).to_header()?;
Expand Down
206 changes: 194 additions & 12 deletions cnd/src/ethereum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,141 @@
#![forbid(unsafe_code)]

pub use ethbloom::{Bloom as H2048, Input};
pub use primitive_types::{H160, H256, U128, U256};
use hex::FromHexError;
pub use primitive_types::U256;
use serde::{Deserialize, Serialize};
use serde_hex::{CompactPfx, SerHex, SerHexSeq, StrictPfx};

pub type Address = H160;
use std::{
fmt,
fmt::{Display, Formatter, LowerHex},
str::FromStr,
};

#[derive(Debug, Default, Copy, Clone, PartialEq, Deserialize)]
pub struct H64(#[serde(with = "SerHex::<StrictPfx>")] [u8; 8]);

#[derive(
Debug, Default, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize,
)]
pub struct Address(#[serde(with = "SerHex::<StrictPfx>")] [u8; 20]);

impl Address {
pub fn from_slice(src: &[u8]) -> Self {
let mut address = Address([0u8; 20]);
address.0.copy_from_slice(src);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can panic if the slice is not long enough, i.e. < 20 bytes. Can you please create a follow-up ticket for auditing the usages of this function with the goal of removing it?

Copy link
Member Author

Choose a reason for hiding this comment

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

follow up: #2412

address
}
}

impl From<[u8; 20]> for Address {
fn from(bytes: [u8; 20]) -> Self {
Address(bytes)
}
}

impl From<Address> for [u8; 20] {
fn from(s: Address) -> Self {
s.0
}
}

impl FromStr for Address {
type Err = FromHexError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
hex::decode(s).map(|v| Address::from_slice(v.as_slice()))
}
}

impl From<Address> for Hash {
Copy link
Contributor

Choose a reason for hiding this comment

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

That looks weird, why do we need to convert between an Address and a Hash?

Copy link
Contributor

Choose a reason for hiding this comment

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

If we currently do this in production code and removing that is non-trivial, can you please record a follow-up issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

Recorded as follow up: #2411

The problem is that the Topic used to search in Events just accepts a Hash specifically, but we also use it for searching for Addresses - we should change this to be generic.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay interesting, I would put this functionality on the specific topic then and not as a generall conversion between addresses and hashes.

fn from(address: Address) -> Self {
let mut h256 = Hash([0u8; 32]);
h256.0[(32 - 20)..32].copy_from_slice(&address.0);
h256
}
}

impl LowerHex for Address {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
if f.alternate() {
write!(f, "0x")?;
}
for i in &self.0[..] {
write!(f, "{:02x}", i)?;
}
Ok(())
}
}

#[derive(
Debug, Default, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize,
)]
pub struct Hash(#[serde(with = "SerHex::<StrictPfx>")] [u8; 32]);

impl From<[u8; 32]> for Hash {
fn from(bytes: [u8; 32]) -> Self {
Hash(bytes)
}
}

impl From<Hash> for [u8; 32] {
fn from(s: Hash) -> Self {
s.0
}
}

impl Hash {
pub fn from_slice(src: &[u8]) -> Self {
let mut h256 = Hash([0u8; 32]);
h256.0.copy_from_slice(src);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, this can panic and we should avoid using this function.

Copy link
Member Author

Choose a reason for hiding this comment

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

follow up: #2412

h256
}

pub fn as_bytes(&self) -> &[u8] {
&self.0
}
}

impl FromStr for Hash {
type Err = FromHexError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
hex::decode(s).map(|v| Hash::from_slice(v.as_slice()))
}
}

impl LowerHex for Hash {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
if f.alternate() {
write!(f, "0x")?;
}
for i in &self.0[..] {
write!(f, "{:02x}", i)?;
}
Ok(())
}
}

impl Display for Hash {
Copy link
Contributor

Choose a reason for hiding this comment

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

Eventually, I would like this Display implementation to not print ... but do what the implementation of LowerHex does.
We can then remove the implementation of LowerHex.

This will likely require a few changes to log statements in our code where we currently use {:x} to print the full transaction hash.

Can be done as a follow-up ticket :)

Copy link
Member Author

Choose a reason for hiding this comment

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

recorded: #2413

fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
write!(f, "0x")?;
for i in &self.0[0..2] {
write!(f, "{:02x}", i)?;
}
write!(f, "…")?;
for i in &self.0[32 - 2..32] {
write!(f, "{:02x}", i)?;
}
Ok(())
}
}

/// "Receipt" of an executed transaction: details of its execution.
#[derive(Debug, Default, Clone, PartialEq, Deserialize)]
pub struct TransactionReceipt {
/// Contract address created, or `None` if not a deployment.
#[serde(rename = "contractAddress")]
pub contract_address: Option<H160>,
pub contract_address: Option<Address>,
/// Logs generated within this transaction.
pub logs: Vec<Log>,
/// Status: either 1 (success) or 0 (failure).
Expand All @@ -34,9 +154,9 @@ impl TransactionReceipt {
#[derive(Debug, Default, Clone, PartialEq, Deserialize)]
pub struct Transaction {
/// Hash
pub hash: H256,
pub hash: Hash,
/// Recipient (None when contract creation)
pub to: Option<H160>,
pub to: Option<Address>,
/// Transfered value
pub value: U256,
/// Input data
Expand All @@ -47,9 +167,9 @@ pub struct Transaction {
#[derive(Debug, Clone, PartialEq, Deserialize)]
pub struct Log {
/// H160
pub address: H160,
pub address: Address,
/// Topics
pub topics: Vec<H256>,
pub topics: Vec<Hash>,
/// Data
pub data: Bytes,
}
Expand All @@ -60,10 +180,10 @@ pub struct Log {
#[derive(Debug, Default, Clone, PartialEq, Deserialize)]
pub struct Block {
/// Hash of the block
pub hash: Option<H256>,
pub hash: Option<Hash>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you record a follow-up issue for making this not an Option? It is so annoying 😬

Copy link
Member Author

Choose a reason for hiding this comment

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

recorded: #2414

/// Hash of the parent
#[serde(rename = "parentHash")]
pub parent_hash: H256,
pub parent_hash: Hash,
/// Logs bloom
#[serde(rename = "logsBloom")]
pub logs_bloom: H2048,
Expand All @@ -85,8 +205,70 @@ impl<T: Into<Vec<u8>>> From<T> for Bytes {

#[cfg(test)]
mod tests {
use super::Log;
use crate::ethereum::TransactionReceipt;
use super::*;

#[test]
fn deserialise_address() {
let json =
serde_json::Value::String("0xc5549e335b2786520f4c5d706c76c9ee69d0a028".to_owned());
let _: Address = Address::deserialize(&json).unwrap();
}

#[test]
fn deserialise_address_when_not_using_reference_to_deserialize_fails() {
// This is due to a bug in serde-jex, keep this test until https://github.com/fspmarshall/serde-hex/pull/8
// is fixed.
let json =
serde_json::Value::String("0xc5549e335b2786520f4c5d706c76c9ee69d0a028".to_owned());

let deserialized = serde_json::from_value::<Address>(json);
matches!(deserialized, Err(_));
}

#[test]
fn from_string_address() {
let json =
serde_json::Value::String("0xc5549e335b2786520f4c5d706c76c9ee69d0a028".to_owned());
let deserialized: Address = Address::deserialize(&json).unwrap();

let from_string = Address::from_str("c5549e335b2786520f4c5d706c76c9ee69d0a028").unwrap();

assert_eq!(from_string, deserialized);
}

#[test]
fn deserialise_hash() {
let json = serde_json::Value::String(
"0x3ae3b6ffb04204f52dee42000e8b971c0f7c2b4aa8dd9455e41a30ee4b31e8a9".to_owned(),
);
let _: Hash = Hash::deserialize(&json).unwrap();
}

#[test]
fn deserialise_hash_when_not_using_reference_to_deserialize_fails() {
// This is due to a bug in serde-jex, keep this test until https://github.com/fspmarshall/serde-hex/pull/8
// is fixed.
let json = serde_json::Value::String(
"0x3ae3b6ffb04204f52dee42000e8b971c0f7c2b4aa8dd9455e41a30ee4b31e8a9".to_owned(),
);

let deserialized = serde_json::from_value::<Hash>(json);
matches!(deserialized, Err(_));
}

#[test]
fn from_string_hash() {
let json = serde_json::Value::String(
"0x3ae3b6ffb04204f52dee42000e8b971c0f7c2b4aa8dd9455e41a30ee4b31e8a9".to_owned(),
);
let deserialized: Hash = Hash::deserialize(&json).unwrap();

let from_string =
Hash::from_str("3ae3b6ffb04204f52dee42000e8b971c0f7c2b4aa8dd9455e41a30ee4b31e8a9")
.unwrap();

assert_eq!(from_string, deserialized);
}

#[test]
fn deserialise_log() {
Expand Down
8 changes: 4 additions & 4 deletions cnd/src/http_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ impl Serialize for Http<identity::Bitcoin> {
}

impl_serialize_type_with_fields!(htlc_location::Bitcoin { "txid" => txid, "vout" => vout });
impl_serialize_http!(crate::ethereum::H160);
impl_serialize_http!(crate::ethereum::Address);
impl_serialize_http!(SwapId);

impl Serialize for Http<SwapProtocol> {
Expand Down Expand Up @@ -489,7 +489,7 @@ mod tests {
asset,
asset::ethereum::FromWei,
bitcoin::PublicKey,
ethereum::{H160, H256, U256},
ethereum::{Address, Hash, U256},
http_api::{Http, HttpAsset, HttpLedger},
swap_protocols::{
ledger::{bitcoin, ethereum, Ethereum},
Expand Down Expand Up @@ -591,7 +591,7 @@ mod tests {
output: vec![],
};
let ethereum_tx = transaction::Ethereum {
hash: H256::repeat_byte(1),
hash: Hash::from([1u8; 32]),
..transaction::Ethereum::default()
};

Expand All @@ -618,7 +618,7 @@ mod tests {
.parse()
.unwrap();

let ethereum_identity = H160::repeat_byte(7);
let ethereum_identity = Address::from([7u8; 20]);

let bitcoin_identity = Http(bitcoin_identity);
let ethereum_identity = Http(ethereum_identity);
Expand Down
Loading