Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fee prediction #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add fee prediction #10

wants to merge 1 commit into from

Conversation

joepetrowski
Copy link
Member

No description provided.

Comment on lines +252 to +261
if (hash) {
return hash;
}

if (partialFee) {
return {
weight,
partialFee
};
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Two questions about this:

  1. It just seems very hacky in general. There must be a more generic way to group return items?
  2. The return will either be { hash; } or { weight; class; partialFee; }. In the latter case, can I group these into a DispatchInfo interface? Also, class of course is reserved, so how do I pick it out as a field of DispatchInfo in line 247?

Choose a reason for hiding this comment

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

I think the root of your problems goes back to your layers of abstraction. At a glance, it looks like you're trying to do too much in this function (sidecarPost). I would suggest creating purpose-built functions, each of which has their own singular return type and each of which calls into the POST helper function. You may need to do some additional refactoring to achieve this, but overall I think this is the way in which you want to be thinking 👍

Choose a reason for hiding this comment

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

Talked with Joe on offline about investigating a generic sidecarPost<T> solution to this problem.

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 with Dan, I would create:

  • sidecarPost<T>, whose goal is only to send a POST request to the sidecar
  • and maybe in submitTransaction and getFeeEstimate, do the if (hash) else ... checks.

In the latter case, can I group these into a DispatchInfo interface? Also, class of course is reserved, so how do I pick it out as a field of DispatchInfo in line 247?

Yeah, I guess you could:

+import { RuntimeDispatchInfo } from '@polkadot/types/interfaces';

-export async function getFeeEstimate(sidecarHost: string, encodedTx: string): Promise<any> {
+export async function getFeeEstimate(sidecarHost: string, encodedTx: string): Promise<RuntimeDispatchInfo> {

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

Successfully merging this pull request may close these issues.

3 participants