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

refactor(configuration): Support Bridgechains & Custom Configurations #105

Merged
merged 4 commits into from Jul 2, 2019

Conversation

Projects
None yet
4 participants
@sleepdefic1t
Copy link
Contributor

commented Jul 1, 2019

The current implementation only allows use with Devnet unless values are manually changed in /src.
This was never fully intended, as such this is submitted as a refactor as opposed to a feature PR.

These changes enable network configuration via the ARK Crypto library api.
(Devnet, Mainnet, Testnet, and Custom Networks (bridgechains).

Specifically this PR does the following:

  • Creates a Configuration class.
  • Creates Fee and Network Manager bases for the Configuration class.
  • Allows passing a Configuration to Transaction Builder with a default value of Devnet & StaticFees (non-breaking).
  • Adds a FeePolicy-type.
  • Adds a Fee class (container for fee policies).
  • Reimplements Static Fees as a FeePolicy-type (breaking).
  • Adds a Networks class (base for preset networks).
  • Updates the TransactionTypes enum.
  • Reimplements and improves the Network and Networks classes (breaking).
  • Updates the Slot class to reflect changes (non-breaking).
  • Updates Transaction-related classes to reflect updates (non-breaking).
  • Adds tests for all changes.
  • Updates the Arduino IDE script.
  • Updates .ino Arduino sketches with Configuration examples.
  • Updates documentation with Configuration examples.
  • Updates the keywords.txt file to reflect changes.
  • Updates the changelog.
  • Updates version to v.0.6.0.

What kind of change does this PR introduce?

  • Refactor

Does this PR introduce a breaking change?

  • Yes
  • No

Does this PR release a new version?

  • Yes
  • No

There are two breaking changes in this PR.
(e.g. Devnet vs Devnet(), and Fees vs Fees::StaticFeePolicy)


The PR fulfills these requirements:

  • It's submitted to the develop branch, not the master branch
  • All tests are passing
  • New/updated tests are included
refactor(configuration): support bridgechain configurations
The current implementation only allows use with Devnet unless values are manually changed in `/src`.

This PR enables network configuration via the public api.
(Devnet, Mainnet, Testnet, and Custom Networks (bridgechains).

Specifically this PR does the following:
- Creates a Configuration class.
- Creates Fee and Network Manager children for the Configuration class.
- Allows passing a `Configuration to` Transaction Builder with a default value of `Devnet` & `StaticFees` (non-breaking).
- Adds a FeePolicy-type.
- Adds a Fee class (container for fee policies).
- Adds a Networks class (container for preset networks).
- Updates the TransactionTypes enum.
- Improves the `Network` class and Network implementions.
- Updates the `Slot` class to reflect changes (non-breaking).
- Updates Transaction-related classes to reflect updates (non-breaking).
- Adds tests for all changes.
- Updates the Arduino IDE script.
- Updates `.ino` Arduino sketches with Configuration examples.
- Updates documentation with Configuration examples.
- Updates the `keywords.txt` file to reflect changes.
- Updates the changelog.
- Updates version to `v.0.6.0`.

Some small changes are technically breaking-changes.
(e.g. `Devnet` vs `Devnet()`, and `Fees` vs `Fees::StaticFeePolicy`)
@codecov-io

This comment has been minimized.

Copy link

commented Jul 1, 2019

Codecov Report

Merging #105 into develop will increase coverage by 0.37%.
The diff coverage is 86.4%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #105      +/-   ##
===========================================
+ Coverage    91.14%   91.52%   +0.37%     
===========================================
  Files           27       30       +3     
  Lines          836      861      +25     
===========================================
+ Hits           762      788      +26     
+ Misses          74       73       -1
Impacted Files Coverage Δ
src/transactions/transaction.cpp 88.94% <100%> (ø) ⬆️
src/include/cpp-crypto/common/configuration.hpp 100% <100%> (ø)
src/common/network.cpp 100% <100%> (ø)
src/utils/slot.cpp 100% <100%> (ø) ⬆️
src/networks/testnet.cpp 100% <100%> (ø)
src/defaults/static_fees.cpp 100% <100%> (ø)
src/managers/network_manager.cpp 100% <100%> (ø)
src/networks/devnet.cpp 100% <100%> (ø)
src/networks/mainnet.cpp 100% <100%> (ø)
src/managers/fee_manager.hpp 100% <100%> (ø)
... and 19 more

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 ed0ab57...b1a8ac5. Read the comment docs.

@faustbrian

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

I will take a look when I am back in a few days but just a quick note on the public API part (disregard if not relevant)

All our crypto packages are being built with the lack of any knowledge about networking as that isn't and shouldn't be of any concern for crypto to work. How the developer decides to feed the configuration to the package is also not the concern of the crypto package, the only concern it has is to receive the configuration in a specific format.

If that configuration came from a remote API, file or function that just generated an object isn't our concern and we shouldn't be involved in that process.

@sleepdefic1t sleepdefic1t referenced this pull request Jul 1, 2019

Merged

chore(arduino): merge changes at #105 for Arduino. #106

4 of 6 tasks complete

sleepdefic1t added some commits Jul 1, 2019

@sleepdefic1t

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

@faustbrian
No worries, I should clarify.
The public API I was referring to is the ARK Crypto library API, not the ARK Client API.

@faustbrian

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

All good then, get paranoid when people submit PRs with the words public API to the crypto packages :trollface:

Will be back normal around thursday/friday and then take a look at the whole PR.

@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

The ci/circleci: build-macos-9-2 job is failing as of 2510281468e9ea2a7bbf41ae56d5611520d65c65. Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

1 similar comment
@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

The ci/circleci: build-macos-9-2 job is failing as of 2510281468e9ea2a7bbf41ae56d5611520d65c65. Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

faustbrian added a commit that referenced this pull request Jul 2, 2019

chore(arduino): merge changes at #105 for Arduino. (#106)
chore(arduino): merge changes at #105 for Arduino.

Co-authored-by: Chris Johnson <chrisjohnsonmail@gmail.com>
Co-authored-by: Brian Faust <faustbrian@users.noreply.github.com>

@faustbrian faustbrian merged commit 00575f7 into ArkEcosystem:develop Jul 2, 2019

6 checks passed

ci/circleci: build-arduino-default Your tests passed on CircleCI!
Details
ci/circleci: build-linux-clang-5 Your tests passed on CircleCI!
Details
ci/circleci: build-linux-default Your tests passed on CircleCI!
Details
ci/circleci: build-linux-gcc7 Your tests passed on CircleCI!
Details
ci/circleci: build-macos-9-2 Your tests passed on CircleCI!
Details
ci/circleci: build-macos-9-3 Your tests passed on CircleCI!
Details

@sleepdefic1t sleepdefic1t deleted the sleepdefic1t:refactor(configuration)/support-bridgechains branch Jul 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.