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

Improve CanonicalAddr interoperability #1463

Merged
merged 4 commits into from
Oct 18, 2022
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
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@ and this project adheres to

## [Unreleased]

### Added

- cosmwasm-std: Add `From` implementations to convert between
`CanonicalAddr`/`Binary` as well as `CanonicalAddr`/`HexBinary` ([#1463]).
- cosmwasm-std: Add `From` implementations to convert `u8` arrays to
`CanonicalAddr` ([#1463]).
- cosmwasm-std: Implement `PartialEq` between `CanonicalAddr` and
`HexBinary`/`Binary` ([#1463]).

[#1463]: https://github.com/CosmWasm/cosmwasm/pull/1463

## [1.1.5] - 2022-10-17

### Added
Expand Down
162 changes: 151 additions & 11 deletions packages/std/src/addresses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::borrow::Cow;
use std::fmt;
use std::ops::Deref;

use crate::binary::Binary;
use crate::{binary::Binary, HexBinary};

/// A human readable address.
///
Expand Down Expand Up @@ -138,24 +138,96 @@ impl<'a> From<&'a Addr> for Cow<'a, Addr> {
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, Hash, JsonSchema)]
pub struct CanonicalAddr(pub Binary);

/// Implement `CanonicalAddr == Binary`
impl PartialEq<Binary> for CanonicalAddr {
fn eq(&self, rhs: &Binary) -> bool {
&self.0 == rhs
}
}

/// Implement `Binary == CanonicalAddr`
impl PartialEq<CanonicalAddr> for Binary {
fn eq(&self, rhs: &CanonicalAddr) -> bool {
self == &rhs.0
}
}

/// Implement `CanonicalAddr == HexBinary`
impl PartialEq<HexBinary> for CanonicalAddr {
fn eq(&self, rhs: &HexBinary) -> bool {
self.as_slice() == rhs.as_slice()
}
}

/// Implement `HexBinary == CanonicalAddr`
impl PartialEq<CanonicalAddr> for HexBinary {
fn eq(&self, rhs: &CanonicalAddr) -> bool {
self.as_slice() == rhs.0.as_slice()
}
}

impl From<&[u8]> for CanonicalAddr {
fn from(source: &[u8]) -> Self {
Self(source.into())
}
}

// Array reference
impl<const LENGTH: usize> From<&[u8; LENGTH]> for CanonicalAddr {
fn from(source: &[u8; LENGTH]) -> Self {
Self(source.into())
}
}
Comment on lines +175 to +180
Copy link
Contributor

Choose a reason for hiding this comment

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

What about a slice?

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean impl From<&[u8]> for CanonicalAddr? This was already implemented before a few lines above this addition.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, my bad!


// Owned array
impl<const LENGTH: usize> From<[u8; LENGTH]> for CanonicalAddr {
fn from(source: [u8; LENGTH]) -> Self {
Self(source.into())
}
}

// Owned vector -> CanonicalAddr
impl From<Vec<u8>> for CanonicalAddr {
fn from(source: Vec<u8>) -> Self {
Self(source.into())
}
}

// CanonicalAddr -> Owned vector
impl From<CanonicalAddr> for Vec<u8> {
fn from(source: CanonicalAddr) -> Vec<u8> {
source.0.into()
}
}

// Owned Binary -> CanonicalAddr
impl From<Binary> for CanonicalAddr {
fn from(source: Binary) -> Self {
Self(source)
}
}

// CanonicalAddr -> Owned Binary
impl From<CanonicalAddr> for Binary {
fn from(source: CanonicalAddr) -> Binary {
source.0
}
}

// Owned HexBinary -> CanonicalAddr
impl From<HexBinary> for CanonicalAddr {
fn from(source: HexBinary) -> Self {
Self(source.into())
}
}

// CanonicalAddr -> Owned HexBinary
impl From<CanonicalAddr> for HexBinary {
fn from(source: CanonicalAddr) -> HexBinary {
source.0.into()
}
}

/// Just like Vec<u8>, CanonicalAddr is a smart pointer to [u8].
/// This implements `*canonical_address` for us and allows us to
/// do `&*canonical_address`, returning a `&[u8]` from a `&CanonicalAddr`.
Expand Down Expand Up @@ -278,8 +350,44 @@ mod tests {
}

#[test]
fn canonical_addr_from_vec_works() {
fn canonical_addr_implements_partial_eq_with_binary() {
let addr = CanonicalAddr::from([1, 2, 3]);
let bin1 = Binary::from([1, 2, 3]);
let bin2 = Binary::from([42, 43]);

assert_eq!(addr, bin1);
assert_eq!(bin1, addr);
assert_ne!(addr, bin2);
assert_ne!(bin2, addr);
}

#[test]
fn canonical_addr_implements_partial_eq_with_hex_binary() {
let addr = CanonicalAddr::from([1, 2, 3]);
let bin1 = HexBinary::from([1, 2, 3]);
let bin2 = HexBinary::from([42, 43]);

assert_eq!(addr, bin1);
assert_eq!(bin1, addr);
assert_ne!(addr, bin2);
assert_ne!(bin2, addr);
}

#[test]
fn canonical_addr_implements_from_array() {
let array = [1, 2, 3];
let addr = CanonicalAddr::from(array);
assert_eq!(addr.as_slice(), [1, 2, 3]);

let array_ref = b"foo";
let addr = CanonicalAddr::from(array_ref);
assert_eq!(addr.as_slice(), [0x66, 0x6f, 0x6f]);
}

#[test]
fn canonical_addr_implements_from_and_to_vector() {
// Into<CanonicalAddr> for Vec<u8>
// This test is a bit pointless because we get Into from the From implementation
let original = vec![0u8, 187, 61, 11, 250, 0];
let original_ptr = original.as_ptr();
let addr: CanonicalAddr = original.into();
Expand All @@ -292,11 +400,9 @@ mod tests {
let addr = CanonicalAddr::from(original);
assert_eq!(addr.as_slice(), [0u8, 187, 61, 11, 250, 0]);
assert_eq!((addr.0).0.as_ptr(), original_ptr, "must not be copied");
}

#[test]
fn canonical_addr_into_vec_works() {
// Into<Vec<u8>> for CanonicalAddr
// This test is a bit pointless because we get Into from the From implementation
let original = CanonicalAddr::from(vec![0u8, 187, 61, 11, 250, 0]);
let original_ptr = (original.0).0.as_ptr();
let vec: Vec<u8> = original.into();
Expand All @@ -311,6 +417,40 @@ mod tests {
assert_eq!(vec.as_ptr(), original_ptr, "must not be copied");
}

#[test]
fn canonical_addr_implements_from_and_to_binary() {
// From<Binary> for CanonicalAddr
let original = Binary::from([0u8, 187, 61, 11, 250, 0]);
let original_ptr = original.as_ptr();
let addr = CanonicalAddr::from(original);
assert_eq!(addr.as_slice(), [0u8, 187, 61, 11, 250, 0]);
assert_eq!((addr.0).0.as_ptr(), original_ptr, "must not be copied");

// From<CanonicalAddr> for Binary
let original = CanonicalAddr::from(vec![7u8, 35, 49, 101, 0, 255]);
let original_ptr = (original.0).0.as_ptr();
let bin = Binary::from(original);
assert_eq!(bin.as_slice(), [7u8, 35, 49, 101, 0, 255]);
assert_eq!(bin.as_ptr(), original_ptr, "must not be copied");
}

#[test]
fn canonical_addr_implements_from_and_to_hex_binary() {
// From<HexBinary> for CanonicalAddr
let original = HexBinary::from([0u8, 187, 61, 11, 250, 0]);
let original_ptr = original.as_ptr();
let addr = CanonicalAddr::from(original);
assert_eq!(addr.as_slice(), [0u8, 187, 61, 11, 250, 0]);
assert_eq!((addr.0).0.as_ptr(), original_ptr, "must not be copied");

// From<CanonicalAddr> for HexBinary
let original = CanonicalAddr::from(vec![7u8, 35, 49, 101, 0, 255]);
let original_ptr = (original.0).0.as_ptr();
let bin = HexBinary::from(original);
assert_eq!(bin.as_slice(), [7u8, 35, 49, 101, 0, 255]);
assert_eq!(bin.as_ptr(), original_ptr, "must not be copied");
}

#[test]
fn canonical_addr_len() {
let bytes: &[u8] = &[0u8, 187, 61, 11, 250, 0];
Expand Down Expand Up @@ -359,17 +499,17 @@ mod tests {

#[test]
fn canonical_addr_implements_hash() {
let alice1 = CanonicalAddr(Binary::from([0, 187, 61, 11, 250, 0]));
let alice1 = CanonicalAddr::from([0, 187, 61, 11, 250, 0]);
let mut hasher = DefaultHasher::new();
alice1.hash(&mut hasher);
let alice1_hash = hasher.finish();

let alice2 = CanonicalAddr(Binary::from([0, 187, 61, 11, 250, 0]));
let alice2 = CanonicalAddr::from([0, 187, 61, 11, 250, 0]);
let mut hasher = DefaultHasher::new();
alice2.hash(&mut hasher);
let alice2_hash = hasher.finish();

let bob = CanonicalAddr(Binary::from([16, 21, 33, 0, 255, 9]));
let bob = CanonicalAddr::from([16, 21, 33, 0, 255, 9]);
let mut hasher = DefaultHasher::new();
bob.hash(&mut hasher);
let bob_hash = hasher.finish();
Expand All @@ -381,9 +521,9 @@ mod tests {
/// This requires Hash and Eq to be implemented
#[test]
fn canonical_addr_can_be_used_in_hash_set() {
let alice1 = CanonicalAddr(Binary::from([0, 187, 61, 11, 250, 0]));
let alice2 = CanonicalAddr(Binary::from([0, 187, 61, 11, 250, 0]));
let bob = CanonicalAddr(Binary::from([16, 21, 33, 0, 255, 9]));
let alice1 = CanonicalAddr::from([0, 187, 61, 11, 250, 0]);
let alice2 = CanonicalAddr::from([0, 187, 61, 11, 250, 0]);
let bob = CanonicalAddr::from([16, 21, 33, 0, 255, 9]);

let mut set = HashSet::new();
set.insert(alice1.clone());
Expand Down
3 changes: 1 addition & 2 deletions packages/std/src/imports.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::vec::Vec;

use crate::addresses::{Addr, CanonicalAddr};
use crate::binary::Binary;
use crate::errors::{RecoverPubkeyError, StdError, StdResult, SystemError, VerificationError};
use crate::import_helpers::{from_high_half, from_low_half};
use crate::memory::{alloc, build_region, consume_region, Region};
Expand Down Expand Up @@ -224,7 +223,7 @@ impl Api for ExternalApi {
}

let out = unsafe { consume_region(canon) };
Ok(CanonicalAddr(Binary(out)))
Ok(CanonicalAddr::from(out))
}

fn addr_humanize(&self, canonical: &CanonicalAddr) -> StdResult<Addr> {
Expand Down
2 changes: 1 addition & 1 deletion packages/std/src/testing/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,7 @@ mod tests {
#[should_panic(expected = "length not correct")]
fn addr_humanize_input_length() {
let api = MockApi::default();
let input = CanonicalAddr(Binary(vec![61; 11]));
let input = CanonicalAddr::from(vec![61; 11]);
api.addr_humanize(&input).unwrap();
}

Expand Down