Skip to content
This repository has been archived by the owner on Mar 13, 2023. It is now read-only.

BridgeMessageId=>TokenMessageId #838

Merged
merged 2 commits into from
Sep 24, 2021
Merged

BridgeMessageId=>TokenMessageId #838

merged 2 commits into from
Sep 24, 2021

Conversation

xiaoch05
Copy link
Contributor

To avoid the type conflict of this update.
paritytech/parity-bridges-common#1152

@xiaoch05
Copy link
Contributor Author

@hackfisher the docs is here https://github.com/darwinia-network/darwinia-common/blob/master/frame/support/src/lib.rs#L103-L107.
and for sub<>sub bridge, it combines laneid and nonce. We leave 4 bytes to indict chainid in the future.

and this message_id also used in mapping-token-factory at https://github.com/darwinia-network/darwinia-bridge-sol/blob/master/contracts/mapping-token/contracts/darwinia/DarwiniaMappingTokenFactory.sol#L154

@hackfisher
Copy link
Contributor

hackfisher commented Sep 23, 2021

Since this is only used in wormhole and token protocol, please rename it to TokenMessageId or TokenBridgeMessageId, and also related traits & methods.

	fn latest_message_id() -> BridgeMessageId {
		let nonce: u64 =
			BridgePangolinMessages::outbound_latest_generated_nonce(PANGORO_PANGOLIN_LANE).into();
		nonce_to_message_id(&PANGORO_PANGOLIN_LANE, nonce)
	}

@github-actions

This comment has been minimized.

Copy link
Contributor

@hackfisher hackfisher left a comment

Choose a reason for hiding this comment

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

LGTM, not sure whether we need to update the types.json for pangolin and pangoro.

@xiaoch05 xiaoch05 changed the title BridgeMessageId=>BridgeMessageUid BridgeMessageId=>TokenMessageId Sep 24, 2021
@xiaoch05
Copy link
Contributor Author

LGTM, not sure whether we need to update the types.json for pangolin and pangoro.

The types.json is updated in ./node/runtime/pangolin/types.json. @AurevoirXavier do we use the same type.json file as pangolin in pangoro?

@aurexav
Copy link
Member

aurexav commented Sep 24, 2021

LGTM, not sure whether we need to update the types.json for pangolin and pangoro.

The types.json is updated in ./node/runtime/pangolin/types.json. @AurevoirXavier do we use the same type.json file as pangolin in pangoro?

Yes, there is no need to update for pangoro.

Copy link
Member

@boundless-forest boundless-forest left a comment

Choose a reason for hiding this comment

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

Seems good. A question feedback left.

@@ -186,7 +186,7 @@
"Erc20": "TokenInfo"
}
},
"BridgeMessageId": "[u8; 16; BridgeMessageId]",
"TokenMessageId": "[u8; 16; TokenMessageId]",
Copy link
Member

Choose a reason for hiding this comment

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

I am curious about this writing style, Can we change it to "TokenMessageId": "[u8; 16]",?

@hackfisher hackfisher merged commit f9d2689 into master Sep 24, 2021
@hackfisher hackfisher deleted the rename-messageid branch September 24, 2021 05:08
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.

4 participants