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

Add proper calculation for ccm fee for the bounce function unit tests #8195

Closed
ishantiw opened this issue Feb 24, 2023 · 3 comments
Closed
Assignees
Milestone

Comments

@ishantiw
Copy link
Contributor

Description

  • Relating to the comment, we have not added a calculation of ccm fee after we call the bounce function in the tests.
  • Add something like (context.ccm.fee - (MIN_FEE_PER_BYTE * CCM_BYTES_LENGTH))to explain why addToOutbox was called with the expected ccm fee.

Which version(s) does this affect? (Environment, OS, etc...)

v6.0.0-alpha.14

@Phanco
Copy link
Contributor

Phanco commented Sep 13, 2023

Digging down into this issue.

After the LIP update LIP#405 and the corresponding code update #8765, the fee is currently 0.

@ishantiw I think this issue can be closed.

@ishantiw
Copy link
Contributor Author

@Phanco Event after the LIP update, I think the tests are still prone to allow a bug.
The problem is that there is no explanation of message fee, that why fee: BigInt(100000000000) should pass and why fee: BigInt(1) should fail, by just giving unreasonably high and low values. These values should be calculated from the sample CCM used in the test in order to have these tests completely automated. For ex, if we have incorrect const minFee = minReturnFeePerByte * BigInt(ccmSize) - randomValue; code in source but in test it will still pass assertion for values fee: BigInt(100000000000) and fee: BigInt(1)

In test https://github.com/LiskHQ/lisk-sdk/blob/dc47a3f92ae7f30e3db8982f3fffb8f2ec481836/framework/test/unit/modules/interoperability/base_cross_chain_update_command.spec.ts#L1537, we simply give the lowest fee but without any explanation of this magic value
https://github.com/LiskHQ/lisk-sdk/blob/dc47a3f92ae7f30e3db8982f3fffb8f2ec481836/framework/test/unit/modules/interoperability/base_cross_chain_update_command.spec.ts#L1539-L1543

Here, we can possibly calculate minFee for a CCM, have it in general for all the test and only for this test we can do something like minFee - BigInt(1) and it should fail and log event discarded.

@Phanco
Copy link
Contributor

Phanco commented Sep 14, 2023

@Phanco Event after the LIP update, I think the tests are still prone to allow a bug. The problem is that there is no explanation of message fee, that why fee: BigInt(100000000000) should pass and why fee: BigInt(1) should fail, by just giving unreasonably high and low values. These values should be calculated from the sample CCM used in the test in order to have these tests completely automated. For ex, if we have incorrect const minFee = minReturnFeePerByte * BigInt(ccmSize) - randomValue; code in source but in test it will still pass assertion for values fee: BigInt(100000000000) and fee: BigInt(1)

In test

https://github.com/LiskHQ/lisk-sdk/blob/dc47a3f92ae7f30e3db8982f3fffb8f2ec481836/framework/test/unit/modules/interoperability/base_cross_chain_update_command.spec.ts#L1537

, we simply give the lowest fee but without any explanation of this magic value
https://github.com/LiskHQ/lisk-sdk/blob/dc47a3f92ae7f30e3db8982f3fffb8f2ec481836/framework/test/unit/modules/interoperability/base_cross_chain_update_command.spec.ts#L1539-L1543

Here, we can possibly calculate minFee for a CCM, have it in general for all the test and only for this test we can do something like minFee - BigInt(1) and it should fail and log event discarded.

Make sense, will work in it 👍 Thank you

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

No branches or pull requests

4 participants