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

feat: add query param to add a tip, partialFee, and priority field to each extrinsic for /node/transaction-pool #931

Merged
merged 29 commits into from
Jun 6, 2022
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
cdb505e
add test response
TarikGul May 25, 2022
202a7e7
update controller to include tip query parameter
TarikGul May 25, 2022
4e3e02a
create tests that include tip query param
TarikGul May 25, 2022
5a055d0
update the return type to include tip
TarikGul May 25, 2022
15468e8
update dervice logic to handle tip inclusion
TarikGul May 25, 2022
d4f6810
update the docs
TarikGul May 25, 2022
e047bea
fix grumble
TarikGul May 25, 2022
6d6e51d
switch the naming in the controller from tip to fee
TarikGul May 27, 2022
84df69d
Merge branch 'master' of github.com:paritytech/substrate-api-sidecar …
TarikGul May 27, 2022
830002d
add partial fee functionality
TarikGul May 27, 2022
3f1fae2
handle priority correctly
TarikGul May 31, 2022
aa6a1c0
fix some linting errors
TarikGul May 31, 2022
beeb5e8
update response type for partialfee
TarikGul May 31, 2022
54ec9c2
set query param as includeFee
TarikGul May 31, 2022
9e48a40
update normal extrinsic response
TarikGul May 31, 2022
39b48c0
add const values to defaultMockApi
TarikGul May 31, 2022
10544ce
add computation logic to calculate priority
TarikGul May 31, 2022
e4873f4
update docs
TarikGul May 31, 2022
42476ae
add test suite for operational transactions
TarikGul May 31, 2022
23b48ba
lint
TarikGul May 31, 2022
007e588
remove commented import
TarikGul Jun 3, 2022
2af47d9
rename maxBlock to maxBlockWeight
TarikGul Jun 3, 2022
c2e8c13
merge master
TarikGul Jun 5, 2022
b782328
add end line
TarikGul Jun 6, 2022
b0302e6
merge master
TarikGul Jun 6, 2022
d9792eb
fix priority type decsription
TarikGul Jun 6, 2022
0068e0d
cleanup small grumbles
TarikGul Jun 6, 2022
88152e7
fix comment
TarikGul Jun 6, 2022
6f05643
fix some comments
TarikGul Jun 6, 2022
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
2 changes: 1 addition & 1 deletion docs/dist/app.bundle.js

Large diffs are not rendered by default.

20 changes: 20 additions & 0 deletions docs/src/openapi-v1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,14 @@ paths:
description: Returns the extrinsics that the node knows of that have not
been included in a block.
operationId: getNodeTransactionPool
paramaters:
- name: includeFee
in: query
description: Boolean representing whether or not to include tips, partialFee, and priority in each extrinsic.
required: false
schema:
type: boolean
default: false
responses:
"200":
description: successful operation
Expand Down Expand Up @@ -2765,6 +2773,18 @@ components:
type: string
format: hex
description: Scale encoded extrinsic.
tip:
type: string
format: unsignedInteger
description: The tip included in the extrinsic. Only included if the query param `includeFee` is set to true.
priority:
type: string
format: unsignedInteger
description: Computed priority of an extrinsic. Only included if the query param `includeFee` is set to true.
partialFee:
type: string
format: unsignedInteger
description: Provided `partialFee` of an extrinsic. Only included if the query param `includeFee` is set to true.
TransactionSuccess:
type: object
properties:
Expand Down
11 changes: 7 additions & 4 deletions src/controllers/node/NodeTransactionPoolController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import AbstractController from '../AbstractController';
* - `pool`: array of
* - `hash`: H256 hash of the extrinsic.
* - `encodedExtrinsic`: Scale encoded extrinsic.
* - `tip`: Tip included into the extrinsic. Available when the `tip` query param is set to true.
*/
export default class NodeTransactionPoolController extends AbstractController<NodeTransactionPoolService> {
constructor(api: ApiPromise) {
Expand All @@ -39,18 +40,20 @@ export default class NodeTransactionPoolController extends AbstractController<No
}

/**
** GET pending extrinsics from the Substrate node.
* GET pending extrinsics from the Substrate node.
*
* @param _req Express Request
* @param req Express Request, accepts the query param `tip`
* @param res Express Response
TarikGul marked this conversation as resolved.
Show resolved Hide resolved
*/
private getNodeTransactionPool: RequestHandler = async (
_req,
{ query: { includeFee } },
res
): Promise<void> => {
const shouldIncludeFee = includeFee === 'true';

NodeTransactionPoolController.sanitizedSend(
res,
await this.service.fetchTransactionPool()
await this.service.fetchTransactionPool(shouldIncludeFee)
);
};
}
61 changes: 55 additions & 6 deletions src/services/node/NodeTransactionPoolService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,17 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
import { sanitizeNumbers } from '../../sanitize/sanitizeNumbers';
import { polkadotRegistryV9190 } from '../../test-helpers/registries';
import {
// blockHash789629,
defaultMockApi,
pendingExtrinsics,
queryInfoBalancesTransfer,
queryInfoCouncilVote,
} from '../test-helpers/mock';
import transactionPoolResponse from '../test-helpers/responses/node/transactionPool.json';
import transactionPoolWithTipResponse from '../test-helpers/responses/node/transactionPoolWithTip.json';
import transactionPoolWithTipOperationalResponse from '../test-helpers/responses/node/transactionPoolWithTipOperational.json';
import { NodeTransactionPoolService } from '.';

const nodeTranstionPoolService = new NodeTransactionPoolService(defaultMockApi);
Expand All @@ -32,10 +37,10 @@ describe('NodeTransactionPoolService', () => {
it('works when ApiPromiseWorks (no txs)', async () => {
expect(
sanitizeNumbers(
await nodeTranstionPoolService
.fetchTransactionPool
await nodeTranstionPoolService.fetchTransactionPool(
// blockHash789629
()
false
)
)
).toStrictEqual({ pool: [] });
});
Expand All @@ -52,14 +57,58 @@ describe('NodeTransactionPoolService', () => {

expect(
sanitizeNumbers(
await nodeTranstionPoolService
.fetchTransactionPool
await nodeTranstionPoolService.fetchTransactionPool(
// blockHash789629
()
false
)
)
).toStrictEqual(transactionPoolResponse);

(defaultMockApi.rpc.author as any).pendingExtrinsics = pendingExtrinsics;
});

it('works when query param `includeFee` is set to true for normal extrinsics', async () => {
// This test does not use the same metadata in defaultMockApi. It changes it to v9190,
// and sets it back to the default value after.
const normalExt = polkadotRegistryV9190.createType(
'Extrinsic',
'0x4d028400d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d0196a6cd1652fc83c449884f67e8f444587b69c5874512f1d746ff6f062a097b2acedfe8d2e07915b4c93cc1c3b48a16ebccc1db8eb810146373ba53c9f42ab48e4500000284d717050300e281b7ec09fb8420ca7ba3fbd627fbe203ff04b2ba0777ae1d8a6942257af0230700e8764817'
);
const pool = polkadotRegistryV9190.createType('Vec<Extrinsic>', [
normalExt,
]);
(defaultMockApi.rpc.author as any).pendingExtrinsics = () =>
Promise.resolve().then(() => pool);

expect(
sanitizeNumbers(
await nodeTranstionPoolService.fetchTransactionPool(true)
)
).toStrictEqual(transactionPoolWithTipResponse);

(defaultMockApi.rpc.author as any).pendingExtrinsics = pendingExtrinsics;
});

it('works when query param `includeFee` is set to true for operational extrinsics', async () => {
const operationalExt = polkadotRegistryV9190.createType(
'Extrinsic',
'0x350284004adf51a47b72795366d52285e329229c836ea7bbfe139dbe8fa0700c4f86fc5601fc44dcd1994c111671b3577b02e391be8aff10f7ccf766f3189859ea343db041779a67f9357cba0ba051f83d63e45e7a88b5e2ca642181592052acd9f4ccc8821501c107000f03f2af187bbc8a4a2b5a28c2a3c2d85bf7e5b1700cbf1207a8e4c1eb7d8e7e4037350301'
);
const pool = polkadotRegistryV9190.createType('Vec<Extrinsic>', [
operationalExt,
]);
(defaultMockApi.rpc.author as any).pendingExtrinsics = () =>
Promise.resolve().then(() => pool);
(defaultMockApi.rpc.payment as any).queryInfo = queryInfoCouncilVote;

expect(
sanitizeNumbers(
await nodeTranstionPoolService.fetchTransactionPool(true)
)
).toStrictEqual(transactionPoolWithTipOperationalResponse);

(defaultMockApi.rpc.author as any).pendingExtrinsics = pendingExtrinsics;
(defaultMockApi.rpc.payment as any).queryInfo = queryInfoBalancesTransfer;
});
});
});
162 changes: 155 additions & 7 deletions src/services/node/NodeTransactionPoolService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,172 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <http://www.gnu.org/licenses/>.

import { DispatchClass, Extrinsic, Weight } from '@polkadot/types/interfaces';
import { u32 } from '@polkadot/types-codec';
import BN from 'bn.js';

import { INodeTransactionPool } from '../../types/responses';
import { AbstractService } from '../AbstractService';

export class NodeTransactionPoolService extends AbstractService {
async fetchTransactionPool(): Promise<INodeTransactionPool> {
/**
* Fetch the transaction pool, and provide relevant extrinsic information.
*
* @param includeFee Whether or not to include the fee's and priority of a extrinsic
* in the transaction pool.
*/
public async fetchTransactionPool(
includeFee: boolean
): Promise<INodeTransactionPool> {
const { api } = this;

const extrinsics = await api.rpc.author.pendingExtrinsics();

const pool = extrinsics.map((ext) => {
if (includeFee) {
const pool = await Promise.all(
extrinsics.map((ext) => this.extractExtrinsicInfo(ext))
);

return {
hash: ext.hash.toHex(),
encodedExtrinsic: ext.toHex(),
pool,
};
});
} else {
return {
pool: extrinsics.map((ext) => {
return {
hash: ext.hash.toHex(),
encodedExtrinsic: ext.toHex(),
};
}),
};
}
}

/**
* Extract all information related to the extrinsic, and compute it's
* priority in the transaction pool.
*
* @param ext Extrinsic we want to provide all the information for.
*/
private async extractExtrinsicInfo(ext: Extrinsic) {
const { api } = this;
const { hash, tip } = ext;
const {
class: c,
partialFee,
weight,
} = await api.rpc.payment.queryInfo(ext.toHex());
const priority = await this.computeExtPriority(ext, c, weight);

return {
pool,
hash: hash.toHex(),
encodedExtrinsic: ext.toHex(),
tip: tip.toString(),
priority: priority,
partialFee: partialFee,
};
}

/**
* We calculate the priority of an extrinsic in the transaction pool depending
* on its dispatch class, ie. 'normal', 'operational', 'mandatory'.
*
* The following formula can summarize the below logic.
* tip * (max_block_{weight|length} / bounded_{weight|length})
*
* Please reference this link for more information
* ref: https://github.com/paritytech/substrate/blob/fe5bf49290d166b9552f65e751d46ec592173ebd/frame/transaction-payment/src/lib.rs#L610
*
* @param ext
* @param c
* @param weight
*/
private async computeExtPriority(
ext: Extrinsic,
dispatchClass: DispatchClass,
weight: Weight
): Promise<string> {
const { api } = this;
const { tip, encodedLength: len } = ext;
const BN_ONE = new BN(1);
const sanitizedClass = this.defineDispatchClassType(dispatchClass);

const { maxBlock } = this.api.consts.system.blockWeights;
TarikGul marked this conversation as resolved.
Show resolved Hide resolved
TarikGul marked this conversation as resolved.
Show resolved Hide resolved
const maxLength: u32 =
this.api.consts.system.blockLength.max[sanitizedClass];
const boundedWeight = BN.min(BN.max(weight.toBn(), BN_ONE), maxBlock);
const boundedLength = BN.min(BN.max(new BN(len), BN_ONE), maxLength);
const maxTxPerBlockWeight = maxBlock.div(boundedWeight);
const maxTxPerBlockLength = maxLength.div(boundedLength);

const maxTxPerBlock = BN.min(maxTxPerBlockWeight, maxTxPerBlockLength);
const saturatedTip = tip.toBn().add(BN_ONE);
const scaledTip = this.maxReward(saturatedTip, maxTxPerBlock);
TarikGul marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

nice, totally missed that that was a closure in substrate


let priority: string;
switch (sanitizedClass) {
case 'normal': {
priority = scaledTip.toString();
break;
}
case 'mandatory': {
priority = scaledTip.toString();
break;
}
case 'operational': {
const { inclusionFee } = await api.rpc.payment.queryFeeDetails(
ext.toHex()
);
const { operationalFeeMultiplier } = this.api.consts.transactionPayment;

if (inclusionFee.isNone) {
// This is an unsigned_extrinsic, and does not have priority
priority = '0';
break;
}

const { baseFee, lenFee, adjustedWeightFee } = inclusionFee.unwrap();
const computedInclusionFee = baseFee.add(lenFee).add(adjustedWeightFee);
const finalFee = computedInclusionFee.add(tip.toBn());
const virtualTip = finalFee.mul(operationalFeeMultiplier);
const scaledVirtualTip = this.maxReward(virtualTip, maxTxPerBlock);

priority = scaledTip.add(scaledVirtualTip).toString();
break;
}
default: {
priority = '0';
break;
}
}

return priority;
}

/**
* Explicitly define the type of class an extrinsic is.
*
* @param c DispatchClass of an extrinsic
*/
private defineDispatchClassType(
c: DispatchClass
): 'normal' | 'mandatory' | 'operational' {
const cString = c.type.toLowerCase();
if (cString === 'normal') return 'normal';
if (cString === 'mandatory') return 'mandatory';
if (cString === 'operational') return 'operational';

// This will never be reached, but is here to satisfy the TS compiler.
return 'normal';
}

/**
* Multiply a value (tip) by its maxTxPerBlock multiplier.
* ref: https://github.com/paritytech/substrate/blob/fe5bf49290d166b9552f65e751d46ec592173ebd/frame/transaction-payment/src/lib.rs#L633
*
* @param val Value to be multiplied by the maxTxPerBlock. Usually a tip.
* @param maxTxPerBlock The minimum value between maxTxPerBlockWeight and maxTxPerBlockLength
*/
private maxReward(val: BN, maxTxPerBlock: BN): BN {
return val.mul(maxTxPerBlock);
}
}
Loading