feat: support for feeByEvent query param which will abstract the fees by events
#970
Conversation
TarikGul
changed the title
feeByEvent query param which will abstract the fees by eventsfeeByEvent query param which will abstract the fees by events
|
|
||
| return { | ||
| partialFee: partialFee.toString(), | ||
| error: 'Could not find a reliable fee within the events data.', |
There was a problem hiding this comment.
I know I suggested returning an error at some point, but I am wondering whether, especially called from the /blocks endpoint, it makes sense or not? I'm not sure how often we'll find the fee in events vs having to use the queryInfo result?
We could choose to acknowledge that we'll use events if possible, but if no events are available, it's not an error. We should tell the user whether we managed to use an event or not, so I wonder about a return type here that is just string | undefined (either we found the fee in events or not), and then in getPartialFeeInfo a return type like:
{
partialFee: string,
partialFeeFromEvent?: boolean
error,
dispatchClass
}
Just a thought tho!
There was a problem hiding this comment.
In regards to the error, we are following our pattern for extrinsics where the error doesn't stop block querying execution, but instead sets the info field to have an error key with the relevant information. I think the thought of having a partialFeeFromEvent is a good one, but probably not applicable here, as it might confuse users, and clutters the API response a bit.
In regards to pattern I mentioned above, we already return an error whenever there is an issue or one of the cases for receiving fees aren't satisfied, so IMO it makes sense to keep the error and not return a fee.
it makes sense or not? I'm not sure how often we'll find the fee in events vs having to use the queryInfo result?
I'll scrap the chain a bit and see after 100,000 blocks if there is an issue.
| // Check Event:Withdraw event for the balances pallet | ||
| const withdrawEvent = this.findEvent(events, 'balances', Event.withdraw); | ||
| if (withdrawEvent.length > 0 && withdrawEvent[0].data) { | ||
| const dataArr = withdrawEvent[0].data.toJSON(); | ||
| if (Array.isArray(dataArr)) { | ||
| const fee = (dataArr as Array<number>)[dataArr.length - 1]; | ||
|
|
||
| // The difference between values is 00.00001% or less so they are alike. | ||
| if (this.areFeesSimilar(new BN(fee), partialFee)) { | ||
| return { | ||
| partialFee: fee.toString(), | ||
| }; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Def looks tidier now; I like it!
|
Is it now reading the estimated fee amount instead of the actual fee？ |
No so nothing changes with the |
Feature:
Query Parameter:
feeByEvent-> Defaults to false. When true it will collect thepartialFeeby the events of each extrinsic.How?
rpc::payment::query_infoin order to retrieve an estimated partialFeeEvent::TransactionPaidFeeis added).a) Look through the Balances::Event::Withdraw event. We than compare the value given in the data to the estimated partialFee to confirm that there is some commonality. In this case we look for a less than
00.00001difference in atomic value. (This is a very arguable approach, and I don't think it holds any real mathematical integrity, i Just used that abritrary value as a way to get this working).b) If the above isn't satisfied we do the same for
Treasury::Event::Deposit, and do the same comparison.c) If the above isnt satisfied we then add up all the
Balances::Event::Depositvalues and compare that to the esimated partialFee.d) if none of those values are satisfied we return an error to log into the
infofield for the extrinsic.Todo
closes: #969 #756
Note: This PR will be followed by another PR to add support for
Event::TransactionPaidFee