-
Notifications
You must be signed in to change notification settings - Fork 13
Add getMessageFeeTokenIDFromCCM
method to Interoperability module
#437
Conversation
@@ -2551,7 +2551,7 @@ def verifyCrossChainMessage(trs: Transaction, ccm: CCM) -> None: | |||
def beforeCrossChainCommandExecution(trs: Transaction, ccm: CCM) -> None: | |||
relayerAddress = sha256(trs.senderPublicKey)[:ADDRESS_LENGTH] | |||
|
|||
messageFeeTokenID = Interoperability.getMessageFeeTokenID(ccm.receivingChainID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question to make sure I understand: That means that our previous protocol was wrong because we are already in the receiving chain while running this code, i.e., ccm.receivingChainID = OWN_CHAIN_ID
, right?
Btw, while checking this I had a look in the getMessageFeeTokenID
function and it seems to me that it does not have the right functionality for sidechains in case it gets as input OWN_CHAIN_ID
. See my comment in LIP 45.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah. Im wondering if using the sending chain is 'secure' in this sense, or if we should try to get it from the receiving chain in case that fails
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that using the sending chain should be fine, since you run the ccm hooks in case you apply it (then you should be the receiving chain) or forward it. In both cases, the sending chain should be the one defining the channel and hence the message fee token ID correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. In the "normal case" of direct channel communication, we are always running this in the receiving chain, so we should be checking the sending chain channel.
For the forwarding it is not that important I guess, this is why we did not pay much attention here: Since all channels between mainchain and sidechains have the same message fee token (no matter if is LSK or anything else in the future), then checking sending/receiving chain should give the same result, so sending chain is also fine. Perhaps we should document that it is very important to maintain this property in the future (i.e., "all channels with mainchain should use the same message fee token"), this is something that auditors were asking I guess..
@@ -1405,6 +1405,13 @@ def getMessageFeeTokenID(chainID: ChainID) -> TokenID: | |||
return channel(chainID).messageFeeTokenID | |||
``` | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment about the old getMessageFeeTokenID
function:
In case the arg chainID = OWN_CHAIN_ID
and OWN_CHAIN_ID
is a sidechain, I guess the intended behavior is to output some error "input chain is own chain", but according to current specs it would just return the messageFeeTokenID used in the channel with mainchain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, which is probably at least misleading. We should probably just return an error as you suggest.
Related PR: LiskArchive/lisk-sdk#8806 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Editorial approval
Closes #436.