feat(jellyfish-transaction): add on-chain governance dftx #1851

delphk · 2022-11-07T07:56:35Z

What this PR does / why we need it:

/kind feature

Which issue(s) does this PR fixes?:

Fixes #1822 and fixes #1890

Additional comments?:

netlify · 2022-11-07T07:57:28Z

✅ Deploy Preview for jellyfishsdk ready!

Name	Link
🔨 Latest commit	`b34bbc8`
🔍 Latest deploy log	https://app.netlify.com/sites/jellyfishsdk/deploys/6386c58b644f1c000849a409
😎 Deploy Preview	https://deploy-preview-1851--jellyfishsdk.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

codeclimate · 2022-11-07T07:57:28Z

Code Climate has analyzed commit b34bbc8 and detected 0 issues on this pull request.

View more on Code Climate.

codecov · 2022-11-07T08:07:43Z

Codecov Report

Base: 89.95% // Head: 94.12% // Increases project coverage by +4.17% 🎉

Coverage data is based on head (b34bbc8) compared to base (256e3ef).
Patch coverage: 90.47% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1851      +/-   ##
==========================================
+ Coverage   89.95%   94.12%   +4.17%     
==========================================
  Files         358      361       +3     
  Lines       10432    10487      +55     
  Branches     1317     1322       +5     
==========================================
+ Hits         9384     9871     +487     
+ Misses       1012      592     -420     
+ Partials       36       24      -12

Impacted Files	Coverage Δ
...nsaction-builder/src/txn/txn_builder_governance.ts	`79.31% <81.81%> (+55.50%)`	⬆️
...h-transaction-builder/src/txn/txn_builder_error.ts	`100.00% <100.00%> (ø)`
...ish-transaction/src/script/dftx/dftx_governance.ts	`100.00% <100.00%> (+19.31%)`	⬆️
packages/jellyfish-api-core/src/category/oracle.ts	`94.28% <0.00%> (-5.72%)`	⬇️
...ackages/jellyfish-api-core/src/category/account.ts	`94.04% <0.00%> (-2.39%)`	⬇️
packages/jellyfish-wallet-classic/src/index.ts	`100.00% <0.00%> (ø)`
packages/jellyfish-wallet-classic/src/classic.ts	`100.00% <0.00%> (ø)`
apps/rich-list-api/src/modules/QueueModule.ts	`100.00% <0.00%> (ø)`
apps/whale-api/src/module.api/poolpair.service.ts	`81.64% <0.00%> (+0.39%)`	⬆️
apps/whale-api/src/module.api/loan.controller.ts	`93.26% <0.00%> (+1.92%)`	⬆️
... and 49 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

github-actions · 2022-11-07T08:12:28Z

Docker build preview for jellyfish/apps is ready!

Built with commit 37f2038

ghcr.io/jellyfishsdk/legacy-api:pr-1851
ghcr.io/jellyfishsdk/ocean-api:pr-1851
ghcr.io/jellyfishsdk/playground-api:pr-1851
ghcr.io/jellyfishsdk/status-api:pr-1851
ghcr.io/jellyfishsdk/whale-api:pr-1851

You can also get an immutable image with the commit hash

ghcr.io/jellyfishsdk/legacy-api:37f2038c8063f9813a2676f4934f17330dfd5ffc
ghcr.io/jellyfishsdk/ocean-api:37f2038c8063f9813a2676f4934f17330dfd5ffc
ghcr.io/jellyfishsdk/playground-api:37f2038c8063f9813a2676f4934f17330dfd5ffc
ghcr.io/jellyfishsdk/status-api:37f2038c8063f9813a2676f4934f17330dfd5ffc
ghcr.io/jellyfishsdk/whale-api:37f2038c8063f9813a2676f4934f17330dfd5ffc

infinia-yzl

LGTM

helloscoopa

LGTM

fuxingloh

With on-chain RPC merged, there are conflicts to be resolved.

cwkang1998

Needs to resolve merge conflict

imkven

LGTM

fuxingloh

Overall LGTM!

fuxingloh · 2022-12-02T01:21:56Z

packages/jellyfish-transaction-builder/src/txn/txn_builder_governance.ts

@@ -12,7 +12,12 @@ export class TxnBuilderGovernance extends P2WPKHTxnBuilder {
   * @returns {Promise<TransactionSegWit>}
   */
  async createCfp (createCfp: CreateCfp, changeScript: Script): Promise<TransactionSegWit> {
-    const creationFee = this.network.name === 'regtest' ? new BigNumber('1') : new BigNumber('10')
+    if (createCfp.nCycles > 100) {


I guess should also check for negative?

fuxingloh · 2022-12-02T01:22:32Z

packages/jellyfish-transaction-builder/src/txn/txn_builder_governance.ts

+        'CreateCfp cycles should be between 0 and 100'
+      )
+    }
+    const creationFee = BigNumber.maximum(10, createCfp.nAmount.multipliedBy(0.01))


Suggested change

const creationFee = BigNumber.maximum(10, createCfp.nAmount.multipliedBy(0.01))

// Creation fee must 1% of the requested amount

const creationFee = BigNumber.maximum(10, createCfp.nAmount.multipliedBy(0.01))

I would clarify that and add more comments/documentation to the method. Since it's an expensive operation that is implied by not explicit.

fuxingloh · 2022-12-02T01:27:56Z

packages/jellyfish-transaction-builder/src/txn/txn_builder_governance.ts

@@ -38,7 +43,16 @@ export class TxnBuilderGovernance extends P2WPKHTxnBuilder {
        'CreateVoc address stack should be empty'
      )
    }
-    const creationFee = this.network.name === 'regtest' ? new BigNumber('5') : new BigNumber('50')
+    let creationFee = new BigNumber('5')


I guess it's the same for these because the fees are implied (edited by the method) but not explicitly defined. Users might not be signing what they initially thought they explicitly defined.

fuxingloh · 2022-12-02T01:29:09Z

Merging first, if any follow-up, we can create an issue.

on-chain governance dftx

04464fd

github-actions bot added area/packages kind/feature New feature request labels Nov 7, 2022

delphk added 7 commits November 16, 2022 11:47

update transaction builder

294d019

fix dftx params

0c5f0de

add on-chain governance dftx WIP

65c83fa

Merge branch 'main' into delphk/dftx-onchain-governance

85fca6d

update for latest docker image changes

92a6283

add tests and other fixes

4956f9b

add test cases

763b6e3

delphk marked this pull request as ready for review November 23, 2022 08:40

delphk requested review from fuxingloh, ivan-zynesis and canonbrother as code owners November 23, 2022 08:40

infinia-yzl previously approved these changes Nov 24, 2022

View reviewed changes

helloscoopa previously approved these changes Nov 24, 2022

View reviewed changes

update for newly added cpp changes

cf4a69c

delphk dismissed stale reviews from helloscoopa and infinia-yzl via cf4a69c November 25, 2022 03:28

delphk requested review from cwkang1998, kodemon and eli-lim as code owners November 25, 2022 03:28

github-actions bot added apps/rich-list-api apps/whale-api area/apps labels Nov 25, 2022

infinia-yzl previously approved these changes Nov 25, 2022

View reviewed changes

delphk added 2 commits November 28, 2022 13:54

Merge branch 'main' into delphk/dftx-onchain-governance

cd0583b

update for new breaking cpp changes

59d14fd

delphk dismissed infinia-yzl’s stale review via 59d14fd November 28, 2022 06:36

github-actions bot removed apps/rich-list-api area/apps apps/whale-api labels Nov 28, 2022

infinia-yzl previously approved these changes Nov 29, 2022

View reviewed changes

infinia-yzl added 2 commits November 29, 2022 10:50

Merge branch 'main' into delphk/dftx-onchain-governance

770824d

Merge branch 'main' into delphk/dftx-onchain-governance

6c9eeae

fuxingloh reviewed Nov 29, 2022

View reviewed changes

cwkang1998 previously approved these changes Nov 29, 2022

View reviewed changes

cwkang1998 reviewed Nov 29, 2022

View reviewed changes

delphk added 2 commits November 30, 2022 10:45

Merge branch 'main' into delphk/dftx-onchain-governance

cc7e50a

remove unused code

b34bbc8

delphk dismissed stale reviews from cwkang1998 and infinia-yzl via b34bbc8 November 30, 2022 02:52

infinia-yzl approved these changes Nov 30, 2022

View reviewed changes

imkven approved these changes Nov 30, 2022

View reviewed changes

helloscoopa approved these changes Dec 1, 2022

View reviewed changes

fuxingloh approved these changes Dec 2, 2022

View reviewed changes

fuxingloh merged commit 7ac010b into main Dec 2, 2022

fuxingloh deleted the delphk/dftx-onchain-governance branch December 2, 2022 01:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(jellyfish-transaction): add on-chain governance dftx #1851

feat(jellyfish-transaction): add on-chain governance dftx #1851

delphk commented Nov 7, 2022 •

edited by fuxingloh

Loading

netlify bot commented Nov 7, 2022 •

edited

Loading

codeclimate bot commented Nov 7, 2022 •

edited

Loading

codecov bot commented Nov 7, 2022 •

edited

Loading

github-actions bot commented Nov 7, 2022 •

edited

Loading

infinia-yzl left a comment

helloscoopa left a comment

fuxingloh left a comment

cwkang1998 left a comment

imkven left a comment

fuxingloh left a comment

fuxingloh Dec 2, 2022

fuxingloh Dec 2, 2022

fuxingloh Dec 2, 2022

fuxingloh commented Dec 2, 2022

	const creationFee = BigNumber.maximum(10, createCfp.nAmount.multipliedBy(0.01))
	// Creation fee must 1% of the requested amount
	const creationFee = BigNumber.maximum(10, createCfp.nAmount.multipliedBy(0.01))

feat(jellyfish-transaction): add on-chain governance dftx #1851

feat(jellyfish-transaction): add on-chain governance dftx #1851

Conversation

delphk commented Nov 7, 2022 • edited by fuxingloh Loading

What this PR does / why we need it:

Which issue(s) does this PR fixes?:

Additional comments?:

netlify bot commented Nov 7, 2022 • edited Loading

✅ Deploy Preview for jellyfishsdk ready!

codeclimate bot commented Nov 7, 2022 • edited Loading

codecov bot commented Nov 7, 2022 • edited Loading

Codecov Report

github-actions bot commented Nov 7, 2022 • edited Loading

infinia-yzl left a comment

Choose a reason for hiding this comment

helloscoopa left a comment

Choose a reason for hiding this comment

fuxingloh left a comment

Choose a reason for hiding this comment

cwkang1998 left a comment

Choose a reason for hiding this comment

imkven left a comment

Choose a reason for hiding this comment

fuxingloh left a comment

Choose a reason for hiding this comment

fuxingloh Dec 2, 2022

Choose a reason for hiding this comment

fuxingloh Dec 2, 2022

Choose a reason for hiding this comment

fuxingloh Dec 2, 2022

Choose a reason for hiding this comment

fuxingloh commented Dec 2, 2022

delphk commented Nov 7, 2022 •

edited by fuxingloh

Loading

netlify bot commented Nov 7, 2022 •

edited

Loading

codeclimate bot commented Nov 7, 2022 •

edited

Loading

codecov bot commented Nov 7, 2022 •

edited

Loading

github-actions bot commented Nov 7, 2022 •

edited

Loading