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

added estimateSmartFee rpc #254

Merged
merged 17 commits into from
May 20, 2021
Merged
Show file tree
Hide file tree
Changes from 6 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
40 changes: 40 additions & 0 deletions packages/jellyfish-api-core/__tests__/category/mining.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { RegTestContainer, MasterNodeRegTestContainer } from '@defichain/testcontainers'
import { ContainerAdapterClient } from '../container_adapter_client'
import waitForExpect from 'wait-for-expect'
import { wallet } from '../../src'

describe('non masternode', () => {
const container = new RegTestContainer()
Expand Down Expand Up @@ -31,6 +32,14 @@ describe('non masternode', () => {
const result = await client.mining.getNetworkHashPerSecond()
expect(result).toBe(0)
})

it('should have an error with estimateSmartFee', async () => {
const result = await client.mining.estimateSmartFee(6)
const errors = (result.errors != null) ? result.errors : []
expect(errors.length).toBeGreaterThan(0)
expect(errors[0]).toEqual('Insufficient data or no feerate found')
expect(result.feerate).toBeUndefined()
})
})

describe('masternode', () => {
Expand Down Expand Up @@ -111,3 +120,34 @@ describe('masternode', () => {
expect(info.warnings).toBe('')
})
})

describe('estimatesmartfees', () => {
const container = new MasterNodeRegTestContainer()
const client = new ContainerAdapterClient(container)

beforeAll(async () => {
await container.start()
await container.waitForReady()
await container.waitForWalletCoinbaseMaturity()
await container.waitForWalletBalanceGTE(10)
await client.wallet.setWalletFlag(wallet.WalletFlag.AVOID_REUSE)
})

afterAll(async () => {
await container.stop()
})

it('should have estimated smart fees', async () => {
await waitForExpect(async () => {
for (let x = 0; x < 200; x++) {
const address = await client.wallet.getNewAddress()
await client.wallet.sendToAddress(address, 0.0001)
}
})

const result = await client.mining.estimateSmartFee(6, 'ECONOMICAL')
expect(result.errors).toBeUndefined()
expect(result.blocks).toBeGreaterThan(0)
expect(result.feerate).toBeGreaterThan(0)
})
})
18 changes: 18 additions & 0 deletions packages/jellyfish-api-core/src/category/mining.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ export class Mining {
async getMiningInfo (): Promise<MiningInfo> {
return await this.client.call('getmininginfo', [], 'number')
}

/**
*
* @param {number} confirmationTarget in blocks (1 - 1008)
* @param {EstimateMode} [estimateMode='CONSERVATIVE'] estimateMode of fees.
fuxingloh marked this conversation as resolved.
Show resolved Hide resolved
* @returns {Promise<SmartFeeEstimation>}
*/
async estimateSmartFee (confirmationTarget: number, estimateMode: EstimateMode = 'CONSERVATIVE'): Promise<SmartFeeEstimation> {
return await this.client.call('estimatesmartfee', [confirmationTarget, estimateMode], 'number')
}
}

/**
Expand Down Expand Up @@ -86,3 +96,11 @@ export interface MasternodeInfo {
mintedblocks?: number
lastblockcreationattempt?: string
}

export interface SmartFeeEstimation {
feerate?: number
errors?: string[]
blocks: number
}

type EstimateMode = 'UNSET' | 'ECONOMICAL' | 'CONSERVATIVE'
Copy link
Contributor

Choose a reason for hiding this comment

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

Currently, everyone put it above, we can follow that also, should use enum also. Enum we can add docs. plain types we can't

You can follow this but have it for yourself:

https://github.com/DeFiCh/jellyfish/blob/ece0fdc6e24e2f9601fc4dfa474a9b6c924d507d/packages/jellyfish-api-core/src/category/wallet.ts#L3-L7

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok sure. I just remembered we had a discussion before about enums so I used types.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you convinced me not to use type for everything. I only use type selectively now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also because of this, enum has JSDoc support but for type there isn't: #252

18 changes: 18 additions & 0 deletions website/docs/jellyfish/api/mining.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,21 @@ interface MasternodeInfo {
lastblockcreationattempt?: string
}
```

## estimateSmartFee

Estimates the approximate fee per kilobyte needed for a transaction

```ts title="client.mining.estimateSmartFee()"
interface mining {
estimateSmartFee (confirmationTarget: number, estimateMode: EstimateMode = 'CONSERVATIVE'): Promise<SmartFeeEstimation>
fuxingloh marked this conversation as resolved.
Show resolved Hide resolved
}

interface SmartFeeEstimation {
feerate?: number
errors?: string[]
blocks: number
}

type EstimateMode = 'UNSET' | 'ECONOMICAL' | 'CONSERVATIVE';
```