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
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions src/util/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,21 @@ export async function getClaimType(sidecarHost: string, address: string): Promis
return claimsType.type;
}

// Submit a transaction over Sidecar to the transaction queue.
export async function submitTransaction(sidecarHost: string, encodedTx: string): Promise<any> {
const endpoint = `${sidecarHost}tx/`;
const submission = await sidecarPost(endpoint, encodedTx);
return submission;
}

// Submit a transaction and ask for a transaction fee estimate. Do not submit the transaction to the
// transaction queue.
export async function getFeeEstimate(sidecarHost: string, encodedTx: string): Promise<any> {
const endpoint = `${sidecarHost}tx/fee-estimate/`;
const dispatchInfo = await sidecarPost(endpoint, encodedTx);
return dispatchInfo;
}

/* Signing utilities */

// Ask the user to supply a signature and wait for the response.
Expand Down Expand Up @@ -235,11 +244,20 @@ async function sidecarPost(url: string, tx: string): Promise<any> {
},
)
.then(({ data }) => data)
.then(({ cause, data, error, hash }) => {
.then(({ cause, data, error, hash, weight, partialFee }) => {
if (cause || error) {
throw new Error(`${cause}: ${error} (${data})`);
}

return hash;
if (hash) {
return hash;
}

if (partialFee) {
return {
weight,
partialFee
};
}
Comment on lines +252 to +261
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> {

});
}