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

Permit transfers of wrapped ERC20 tokens between different addresses #329

Merged
merged 12 commits into from
Sep 23, 2022
Merged
20 changes: 15 additions & 5 deletions shared/src/ledger/eth_bridge/bridge_pool_vp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use eyre::eyre;
use crate::ledger::eth_bridge::storage::bridge_pool::{
get_pending_key, is_protected_storage, BRIDGE_POOL_ADDRESS,
};
use crate::ledger::native_vp::{Ctx, NativeVp};
use crate::ledger::native_vp::{Ctx, NativeVp, StorageReader};
use crate::ledger::storage::{DBIter, StorageHasher, DB};
use crate::proto::SignedTxData;
use crate::types::address::{xan, Address, InternalAddress};
Expand Down Expand Up @@ -58,8 +58,18 @@ where
/// associated with an address
fn account_balance_delta(&self, address: &Address) -> Option<SignedAmount> {
let account_key = balance_key(&xan(), address);
let before: Amount = self.ctx.read_pre_value(&account_key)?;
let after: Amount = self.ctx.read_post_value(&account_key)?;
let before: Amount = (&self.ctx)
.read_pre_value(&account_key)
.unwrap_or_else(|error| {
tracing::warn!(?error, %account_key, "reading pre value");
None
})?;
let after: Amount = (&self.ctx)
.read_post_value(&account_key)
.unwrap_or_else(|error| {
tracing::warn!(?error, %account_key, "reading post value");
None
})?;
batconjurer marked this conversation as resolved.
Show resolved Hide resolved
if before > after {
Some(SignedAmount::Negative(before - after))
} else {
Expand Down Expand Up @@ -117,11 +127,11 @@ where
// but that will be a separate PR.
let pending_key = get_pending_key();
let pending_pre: HashSet<PendingTransfer> =
self.ctx.read_pre_value(&pending_key).ok_or(eyre!(
(&self.ctx).read_pre_value(&pending_key)?.ok_or(eyre!(
Copy link
Collaborator

Choose a reason for hiding this comment

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

hm why do we need the explicit borrow here, on ctx? was the compiler (or clippy) complaining about something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's because the trait this method comes from is implemented only for &Ctx and not Ctx

impl<'a, DB, H, CA> StorageReader for &Ctx<'a, DB, H, CA>

If there is a nice way to implement for both Ctx and &Ctx without too much duplication, I'd like to add it (though not in this PR) for ergonomics. (This StorageReader thing actually needs to be contributed back to main or may even be able to be replaced by the new native storage traits coming in soon)

"The bridge pool transfers are missing from storage"
))?;
let pending_post: HashSet<PendingTransfer> =
self.ctx.read_post_value(&pending_key).ok_or(eyre!(
(&self.ctx).read_post_value(&pending_key)?.ok_or(eyre!(
"The bridge pool transfers are missing from storage"
))?;
if !pending_post.contains(&transfer) {
Expand Down
41 changes: 41 additions & 0 deletions shared/src/ledger/eth_bridge/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,44 @@ use crate::types::storage::{Key, KeySeg};
pub fn prefix() -> Key {
Key::from(ADDRESS.to_db_key())
}

/// Returns whether a key belongs to this account or not
pub fn is_eth_bridge_key(key: &Key) -> bool {
matches!(key.segments.get(0), Some(first_segment) if first_segment == &ADDRESS.to_db_key())
}

#[cfg(test)]
mod test {
use super::*;
use crate::types::address;

#[test]
fn test_is_eth_bridge_key_returns_true_for_eth_bridge_address() {
let key = Key::from(super::ADDRESS.to_db_key());
assert!(is_eth_bridge_key(&key));
}

#[test]
fn test_is_eth_bridge_key_returns_true_for_eth_bridge_subkey() {
let key = Key::from(super::ADDRESS.to_db_key())
.push(&"arbitrary key segment".to_owned())
.expect("Could not set up test");
assert!(is_eth_bridge_key(&key));
}

#[test]
fn test_is_eth_bridge_key_returns_false_for_different_address() {
let key =
Key::from(address::testing::established_address_1().to_db_key());
assert!(!is_eth_bridge_key(&key));
}

#[test]
fn test_is_eth_bridge_key_returns_false_for_different_address_subkey() {
let key =
Key::from(address::testing::established_address_1().to_db_key())
.push(&"arbitrary key segment".to_owned())
.expect("Could not set up test");
assert!(!is_eth_bridge_key(&key));
}
Comment on lines +30 to +52
Copy link
Collaborator

Choose a reason for hiding this comment

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

good candidates for a proptest :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will have a go at this (though not in this PR)

}
Loading