planetarium / NineChronicles.EthBridge

NineChronicles ⬌ Ethereum bridge
https://planetarium.github.io/NineChronicles.EthBridge/
GNU Affero General Public License v3.0
11 stars 12 forks source link

feat:update fee policy #270

Closed shkangr closed 11 months ago

shkangr commented 11 months ago

Update Fee Policy

Set Fee per Amount Range.

Applied Two Amount Range. ( Keep Base Fee Policy )

Bridge fee
<1,000 NCG: 10 NCG fixed
≥1,000 NCG: 1% base fee
>10,000 NCG: 1% base fee for total amount + 2% additional surcharge for the amount exceeding 10,000 NCG

Local Test Cases

case1: 500 NCG Transfer -> 490 NCG ( 10 Base Fee )
case2: 1000 NCG Transfer -> 990 NCG ( 10 Base Fee )

case3: 8000 NCG Transfer -> 7920 NCG ( 1% Fee for Range1 )
case4: 10000 NCG Transfer -> 99000 NCG ( 1% Fee for Range1 )

case5: 12000 NCG Transfer -> 11840 NCG ( Fee 160 = 1% Fee + 2% fee for 2000 )
case6: 20000 NCG Transfer -> 19600 NCG ( Fee 400 = 1% Fee + 2% fee for 10000 )
codecov[bot] commented 11 months ago

Codecov Report

Merging #270 (dfee64b) into main (3b52b29) will increase coverage by 1.83%. Report is 2 commits behind head on main. The diff coverage is 100.00%.

:exclamation: Current head dfee64b differs from pull request most recent head c48ee9a. Consider uploading reports for the commit c48ee9a to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #270 +/- ## ========================================== + Coverage 92.00% 93.83% +1.83% ========================================== Files 23 23 Lines 425 422 -3 Branches 63 62 -1 ========================================== + Hits 391 396 +5 + Misses 24 17 -7 + Partials 10 9 -1 ``` | [Files](https://app.codecov.io/gh/planetarium/NineChronicles.EthBridge/pull/270?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=planetarium) | Coverage Δ | | |---|---|---| | [bridge/src/observers/nine-chronicles.ts](https://app.codecov.io/gh/planetarium/NineChronicles.EthBridge/pull/270?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=planetarium#diff-YnJpZGdlL3NyYy9vYnNlcnZlcnMvbmluZS1jaHJvbmljbGVzLnRz) | `91.45% <100.00%> (+1.37%)` | :arrow_up: | | [bridge/src/policies/exchange-fee-ratio.ts](https://app.codecov.io/gh/planetarium/NineChronicles.EthBridge/pull/270?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=planetarium#diff-YnJpZGdlL3NyYy9wb2xpY2llcy9leGNoYW5nZS1mZWUtcmF0aW8udHM=) | `100.00% <100.00%> (+50.00%)` | :arrow_up: |
Junyong-Suh commented 11 months ago

@shkangr haven't looked thoroughly but are we testing below cases?

-1, 0, 1, 0.0000000001, -0.000000000001, " ", " -1", " 0", -Integer.MAX, Integer.MAX, 99, 100, 101, 999, 1000, 10001, 9999, 10000, 10001, 99999, 100000, 1000001

do we also force the minimum amount? i.e., reject the request with less than 100 NCG.

shkangr commented 11 months ago

@Junyong-Suh

case1.  -1 NCG : transaction failed ( don't transfer NCG )
case2.  0 NCG : transaction failed ( don't transfer NCG )
case3.  0.0000000001 NCG : transaction failed, Invalid Format error at GQL ( don't transfer NCG )
case4.  -0.0000000001 NCG : transaction failed, Invalid Format error at GQL ( don't transfer NCG )
case5.  " " NCG : transaction failed, Invalid Format error at GQL ( don't transfer NCG )

I expect it'll works like below cases.

case1.  99 NCG : Refund NCG. Since amount is less than NCG_MINIMUM ( don't transfer NCG to WNCG )
case2.  100, 101, 999 NCG : Base Fee Range
case3.  1000, 9999, 10000 NCG : 1% Fee Range
case4.  10001, 99999, 100000 NCG : 2% Fee Range
case5.  100001 NCG : Transfer 100000 AND Refund ( dont transfer NCG -> NCG ) for left 1 NCG
Junyong-Suh commented 11 months ago

@shkangr @lounlee shouldn't reject the whole transfer instead of sending the partial?

case5.  100001 NCG : Transfer 100000 AND Refund ( dont transfer NCG -> NCG ) for left 1 NCG
shkangr commented 11 months ago

@Junyong-Suh Currently, doesn't reject the whole transfer. If a user try to transfer Amount over NCG_MAXIMUM, bridge transfer amount of NCG_MAXIMUM NCG, and refund ( back to user ) for the left amount NCG.

We have 24 hours rules, so user have to re-transfer after the transaction before.

If we have to appliy new policy ( reject the whole transfer instead of sending the partial ), additional works are needed.

Junyong-Suh commented 11 months ago

@shkangr i see. if the current process is to send as much as possible and refund the rest, that should be good. any thoughts @lounlee ?

pull-request-quantifier-deprecated[bot] commented 11 months ago

This PR has 320 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

``` Label : Large Size : +176 -144 Percentile : 72% Total files changed: 7 Change summary by file extension: .ts : +176 -144 ``` > Change counts above are quantified counts, based on the [PullRequestQuantifier customizations](https://github.com/microsoft/PullRequestQuantifier/blob/main/docs/prquantifier-yaml.md).

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a balance between between PR complexity and PR review overhead. PRs within the optimal size (typical small, or medium sized PRs) mean: - Fast and predictable releases to production: - Optimal size changes are more likely to be reviewed faster with fewer iterations. - Similarity in low PR complexity drives similar review times. - Review quality is likely higher as complexity is lower: - Bugs are more likely to be detected. - Code inconsistencies are more likely to be detected. - Knowledge sharing is improved within the participants: - Small portions can be assimilated better. - Better engineering practices are exercised: - Solving big problems by dividing them in well contained, smaller problems. - Exercising separation of concerns within the code changes. #### What can I do to optimize my changes - Use the PullRequestQuantifier to quantify your PR accurately - Create a context profile for your repo using the [context generator](https://github.com/microsoft/PullRequestQuantifier/releases) - Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the `Excluded` section from your `prquantifier.yaml` context profile. - Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your `prquantifier.yaml` context profile. - Only use the labels that matter to you, [see context specification](./docs/prquantifier-yaml.md) to customize your `prquantifier.yaml` context profile. - Change your engineering behaviors - For PRs that fall outside of the desired spectrum, review the details and check if: - Your PR could be split in smaller, self-contained PRs instead - Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR). #### How to interpret the change counts in git diff output - One line was added: `+1 -0` - One line was deleted: `+0 -1` - One line was modified: `+1 -1` (git diff doesn't know about modified, it will interpret that line like one addition plus one deletion) - Change percentiles: Change characteristics (addition, deletion, modification) of this PR in relation to all other PRs within the repository.


Was this comment helpful? :thumbsup:  :ok_hand:  :thumbsdown: (Email) Customize PullRequestQuantifier for this repository.