Skip to content
This repository has been archived by the owner on Oct 22, 2024. It is now read-only.

Register token with inbound fixture #106

Merged
merged 7 commits into from
Jan 26, 2024

Conversation

yrong
Copy link

@yrong yrong commented Jan 23, 2024

No description provided.

@@ -100,7 +100,7 @@ pub mod accounts {
pub const CHARLIE: &str = "Charlie";
pub const DAVE: &str = "Dave";
pub const EVE: &str = "Eve";
pub const FERDIE: &str = "Ferdei";
pub const FERDIE: &str = "Ferdie";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice! :)

Comment on lines +203 to +214
EthereumBeaconClient::store_execution_header(
register_asset_message.message.proof.block_hash,
register_asset_message.execution_header,
0,
H256::default(),
);

EthereumInboundQueue::submit(
RuntimeOrigin::signed(BridgeHubRococoSender::get()),
register_asset_message.message,
)
.unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm I'm not sure this change is a good one. What do you think is the benefit of changing this? I feel like the previous way is clearer to read and understand what is happening, imo.

Copy link
Author

Choose a reason for hiding this comment

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

To not expose the internal implementation details to the tests.
More context in Snowfork/snowbridge#1115 (comment)

@yrong yrong merged commit e151ad1 into snowbridge Jan 26, 2024
4 of 10 checks passed
@yrong yrong deleted the ron/refactoring-inbound-fixture branch January 26, 2024 01:08
claravanstaden pushed a commit that referenced this pull request Jan 26, 2024
* Register token with inbound fixture

* Import snowbridge-pallet-inbound-queue-fixtures

* Cleanup Cargo.toml

* Update emulated test

* Fix emulated test

* Rename to register_token
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants