Basilisk-node icon indicating copy to clipboard operation
Basilisk-node copied to clipboard

Update/Add integration test to verify "fee transfer to treasury"

Open enthusiastmartin opened this issue 3 years ago • 3 comments

Looks good. There is probably no documentation requiring changes. But I'm not sure about tests. Do we want to add some tests? The problem with testing is that I'm not sure how we can call a TX and charge a fee for it without using dedicated struct for handling fees (ChargeTransactionPayment, as used in the tests in the TX multi payment pallet).

i believe an integration test would be quite useful for this feature. Or update existing one to check if fees are correctly transferred to treasury.

Originally posted by @enthusiastmartin in https://github.com/galacticcouncil/Basilisk-node/issues/493#issuecomment-1156197670

enthusiastmartin avatar Jun 15 '22 09:06 enthusiastmartin

@enthusiastmartin I will need some help with this because I have no idea how to test it.

Roznovjak avatar Jun 15 '22 13:06 Roznovjak

I have already updated the tests to check that the fees end up in the treasury account in the weight trader PR, actually. https://github.com/galacticcouncil/Basilisk-node/pull/443/files#diff-6d47daf1b45b7588537ae4e0d013dbbce1547edf30ab483a14a0ea3229dce2c9L132-R140

apopiak avatar Jun 16 '22 14:06 apopiak

I have already updated the tests to check that the fees end up in the treasury account in the weight trader PR, actually. https://github.com/galacticcouncil/Basilisk-node/pull/443/files#diff-6d47daf1b45b7588537ae4e0d013dbbce1547edf30ab483a14a0ea3229dce2c9L132-R140

Nice.

It would be also great to have a test to verify that also fee in native asset goes to treasury.

@Roznovjak could you try? you can take inspiration of the existing tests but modify to use native asset to pay fees with.

enthusiastmartin avatar Jun 16 '22 18:06 enthusiastmartin