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

jellyfish-core protocol adapter and error handling #41

Merged
merged 18 commits into from
Mar 9, 2021
Merged

Conversation

fuxingloh
Copy link
Contributor

What kind of PR is this?:

/kind feature

What this PR does / why we need it:

This pull request implements the jellyfish-core abstraction for protocol adapting and error handling.

JellyfishClient being an abstract class allows the ability to adapt to any protocol when introduced, ws, https etc while being simple to use. This implementation structure can be observed in ContainerAdapterClient where it is used to test jellyfish-core implementations.

RPC categories are grouped into jellyfish-core/src/category/*.ts e.g. category/mining.ts this allows a protocol agnostic implementation of the RPC. All concerns are grouped within one ts file for better developer experience of browsing and maintaining code. (might refactor this in the future, but we can leave it as it is now)

JellyfishError encapsulate RPC errors from DeFiChain within a structure. This allow for instancesof or type of error handling with rich structure.

Which issue(s) does this PR fixes?:

Fixes #16 and #19

Additional comments?:

Also made changes to testcontainers:

  1. removed json-bigint in favour of a new cleaner implementation that is to be introduced
  2. added a raw container.post(string): string method for test adapting

@defichain-bot defichain-bot added the kind/feature New feature request label Mar 8, 2021
@github-actions
Copy link

github-actions bot commented Mar 8, 2021

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
packages/jellyfish/dist/jellyfish.umd.js 102 B (0%) 10 ms (0%) 75 ms (+8.68% 🔺) 85 ms

@codecov
Copy link

codecov bot commented Mar 8, 2021

Codecov Report

Merging #41 (3ca0441) into main (da91feb) will increase coverage by 1.18%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #41      +/-   ##
==========================================
+ Coverage   91.83%   93.02%   +1.18%     
==========================================
  Files           8       10       +2     
  Lines         147      172      +25     
  Branches       18       19       +1     
==========================================
+ Hits          135      160      +25     
  Misses         12       12              
Impacted Files Coverage Δ
...llyfish-core/__tests__/container_adapter_client.ts 100.00% <100.00%> (ø)
packages/jellyfish-core/src/category/mining.ts 100.00% <100.00%> (ø)
packages/jellyfish-core/src/core.ts 100.00% <100.00%> (ø)
packages/testcontainers/src/chains/container.ts 87.36% <100.00%> (+0.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da91feb...70a521b. Read the comment docs.

@fuxingloh fuxingloh marked this pull request as ready for review March 8, 2021 03:39
@fuxingloh fuxingloh linked an issue Mar 8, 2021 that may be closed by this pull request
isoperator: boolean
masternodeid?: string
masternodeoperator?: string
masternodestate?: 'PRE_ENABLED' | 'ENABLED' | 'PRE_RESIGNED' | 'RESIGNED' | 'PRE_BANNED' | 'BANNED'
Copy link
Member

Choose a reason for hiding this comment

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

Nice. I need to add this on my masternode PR (I only have enabled, pre_resigned and resigned)

@fuxingloh fuxingloh merged commit e2366db into main Mar 9, 2021
@fuxingloh fuxingloh deleted the fuxingloh/core branch March 9, 2021 01:36
eli-lim pushed a commit that referenced this pull request Mar 27, 2022
* added better Create Pull Request body

* tested hex_encoder

* added module

* rename HexEncoder method

* updated readme and module

* added block.ts with test

* added raw.block.ts with test

* added script.activity.ts with test

* added the rest without testing

* fixed ci.yml script
canonbrother pushed a commit that referenced this pull request Jun 1, 2022
* added better Create Pull Request body

* tested hex_encoder

* added module

* rename HexEncoder method

* updated readme and module

* added block.ts with test

* added raw.block.ts with test

* added script.activity.ts with test

* added the rest without testing

* fixed ci.yml script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implementation core with error handling and protocol adapter Promise based client
4 participants