Change Coin to (de)serailize from and to string(CRO-125) by leejw51crypto · Pull Request #168 · crypto-com/thaler

leejw51crypto · 2019-06-28T08:28:39Z

this is for client rpc coin serialization
keep Coin as originally,
and use String just communicating with angular side

Bumps [quickcheck](https://github.com/BurntSushi/quickcheck) from 0.8.3 to 0.8.5. - [Release notes](https://github.com/BurntSushi/quickcheck/releases) - [Commits](BurntSushi/quickcheck@0.8.3...0.8.5) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Problem: latest parity/scale-codec failed to compile (CRO-161) crypto-com#120 Solution: converted usize fields (in datatypes that use scale-codec) to u64

Bumps [protobuf](https://github.com/stepancheg/rust-protobuf) from 2.5.0 to 2.6.0. **This update includes security fixes.** - [Release notes](https://github.com/stepancheg/rust-protobuf/releases) - [Changelog](https://github.com/stepancheg/rust-protobuf/blob/master/CHANGELOG.md) - [Commits](stepancheg/rust-protobuf@v2.5.0...v2.6.0) Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

…ithdraw unbonded stake transactions (CRO-174) add create_unbond_stake_transaction writing unit-test add unit test

remove get_ecdsa_witness remove Secp256k1 call UnboundTx::new add signer for utxo signing use signer in create_deposit_bonded_stake_transaction

…etwork operation tx generation(CRO-176)

tomtau

it would good to remove the irrelevant changes + reformulate the PR / commit message as to what problem this is trying to solve, as per patch requirements:

a patch SHOULD be a minimal and accurate answer to exactly one identified and agreed problem

afaik the original problem was something like:

Problem: javascript client frontends are unable to parse large numbers in JSON

there are different ways to solve it:

return string in client RPC APIs
change coin's serde implementations
return two numbers in client RPC APIs (first div number_of_decimals, second mod number_of_decimals)

the PR should pick the minimal and accurate one -- probably no. 2 is more accurate solution? @calvinaco

tomtau · 2019-07-02T01:24:23Z

chain-core/src/init/coin.rs

 type CoinResult = Result<Coin, CoinError>;

 impl Coin {
+    pub fn value(&self) -> u64 {


Perhaps as impl Into<u64> for Coin?

tomtau · 2019-07-02T01:25:06Z

client-common/src/tendermint/client.rs

    fn broadcast_transaction(&self, transaction: &[u8]) -> Result<()>;
+
+    /// Get staked-state from the staked stake address
+    fn get_account(&self, staked_state_address: &[u8]) -> Result<StakedState>;


seems this change isn't related to the coin deserialisation?

tomtau · 2019-07-02T01:26:06Z

client-common/src/tendermint/rpc_client.rs

            .map(|_| ())
    }
+
+    fn get_account(&self, staked_state_address: &[u8]) -> Result<StakedState> {


not related to coin serialisation

tomtau · 2019-07-02T01:26:19Z

client-index/src/index/default_index.rs

            Ok(())
        }
+
+        fn get_account(&self, staked_state_address: &[u8]) -> Result<StakedState> {


not related to coin serialisation

tomtau · 2019-07-02T01:26:32Z

client-network/src/network_ops.rs

 /// Interface for performing network operations on Crypto.com Chain
 pub trait NetworkOpsClient {
+    /// fetch current StakedState
+    fn get_staked_state_account(


not related to coin serialisation

tomtau · 2019-07-02T01:26:41Z

client-network/src/network_ops/default_network_ops_client.rs

    S: Signer,
+    C: Client,
 {
+    fn get_staked_state_account(


not related to coin serialisation

tomtau · 2019-07-02T01:29:06Z

client-rpc/src/wallet_rpc.rs


    #[rpc(name = "wallet_balance")]
-    fn balance(&self, request: WalletRequest) -> jsonrpc_core::Result<Coin>;
+    fn balance(&self, request: WalletRequest) -> jsonrpc_core::Result<String>;


any reason why not change the serde implementations as in @calvinaco 's old PR https://github.com/crypto-com/chain/pull/122/files#diff-8fb93dcdc2ace21f62aa8ef871ba2102 ?

got it
i'll start from calvain' pr again.

calvinaco · 2019-07-02T03:40:16Z

@leejw51crypto I prefer to change the (De)Serialization of Coin because

in the future we will introduce new RPC call, if we change Coin we will ensure the serialization is applied automatically elsewhere
If we provide other means of API, the serialization will also work.

tomtau · 2019-07-02T04:36:28Z

closing this PR, as it'll be started off from the previous one: #168 (comment)

441: Bump ethbloom from 0.7.0 to 0.8.0 r=devashishdxt a=dependabot-preview[bot] Bumps [ethbloom](https://github.com/paritytech/parity-common) from 0.7.0 to 0.8.0. <details> <summary>Commits</summary> - [`1e9f990`](paritytech/parity-common@1e9f990) update rand to 0.7 (breaking change) ([#217](https://github-redirect.dependabot.com/paritytech/parity-common/issues/217)) - [`f884d77`](paritytech/parity-common@f884d77) Fix deprecation warning ([#168](https://github-redirect.dependabot.com/paritytech/parity-common/issues/168)) - [`b0ec18d`](paritytech/parity-common@b0ec18d) Fix typos ([#229](https://github-redirect.dependabot.com/paritytech/parity-common/issues/229)) - [`28781a0`](paritytech/parity-common@28781a0) Bump parking_lot version ([#227](https://github-redirect.dependabot.com/paritytech/parity-common/issues/227)) - [`bf0ba84`](paritytech/parity-common@bf0ba84) [kvdb-web] bump futures-preview to 0.3.0-alpha.18 ([#225](https://github-redirect.dependabot.com/paritytech/parity-common/issues/225)) - [`31b6479`](paritytech/parity-common@31b6479) [kvdb-web] indexeddb implementation ([#202](https://github-redirect.dependabot.com/paritytech/parity-common/issues/202)) - [`085d18b`](paritytech/parity-common@085d18b) [kvdb-memorydb] Migrated code to 2018 edition, updated parking_lot ([#222](https://github-redirect.dependabot.com/paritytech/parity-common/issues/222)) - [`baf1df9`](paritytech/parity-common@baf1df9) Migrated code to 2018 edition, updated dependencies ([#221](https://github-redirect.dependabot.com/paritytech/parity-common/issues/221)) - [`6a580c1`](paritytech/parity-common@6a580c1) Bump version ([#219](https://github-redirect.dependabot.com/paritytech/parity-common/issues/219)) - [`67a9e7d`](paritytech/parity-common@67a9e7d) Find PendingIterator in Transaction Pool ([#218](https://github-redirect.dependabot.com/paritytech/parity-common/issues/218)) - Additional commits viewable in [compare view](paritytech/parity-common@ethbloom-v0.7.0...ethbloom-v0.8.0) </details> <br /> [![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=ethbloom&package-manager=cargo&previous-version=0.7.0&new-version=0.8.0)](https://dependabot.com/compatibility-score.html?dependency-name=ethbloom&package-manager=cargo&previous-version=0.7.0&new-version=0.8.0) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language - `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com): - Update frequency (including time of day and day of week) - Pull request limits (per update run and/or open at any time) - Out-of-range updates (receive only lockfile updates, if desired) - Security updates (receive only security updates, if desired) Finally, you can contact us by mentioning @dependabot. </details> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>

dependabot-preview bot and others added 30 commits June 12, 2019 14:53

Merge Latest

111a88f

Reformat

cdedba2

Problem: CRO-168 Coin Add Overflow Solution: Add checked_add

8044e65

Merge remote-tracking branch 'upstream/master'

92bd253

Merge remote-tracking branch 'upstream/master'

7a5d285

Problem: CRO-168 Coin Add Overflow Solution: Add checked_sub

3f0a095

Add Unitest For Coin Add Subtract

5ef120f

Merge remote-tracking branch 'upstream/master'

aa1474b

Change To Normal Assert

47662e6

Merge remote-tracking branch 'upstream/master'

69fa29d

Problem: client can't create deposit bonded stake, unbond stake and w…

e1d6357

…ithdraw unbonded stake transactions (CRO-174) add create_unbond_stake_transaction writing unit-test add unit test

Merge remote-tracking branch 'upstream/master'

f4ae6bc

Merge branch 'master' into tx

3bfcadc

tidy up source code

42da4f7

remove return

0252851

remove get_ecdsa_witness

65f0421

remove Secp256k1

77f31bc

call UnboundTx::new

6936d90

add signer for utxo signing

e2e1726

use signer in create_deposit_bonded_stake_transaction

918515c

remove get_ecdsa_witness remove Secp256k1 call UnboundTx::new add signer for utxo signing use signer in create_deposit_bonded_stake_transaction

merge

e76ae15

Merge remote-tracking branch 'upstream/master'

3d815f0

fix unit test

c819f49

fix todo

fdb4536

remove secp256k1zkp

df269ea

Merge remote-tracking branch 'upstream/master'

43bf5e8

Problem: add current tx nonce query for the current staked state in n…

bd753c7

…etwork operation tx generation(CRO-176)

remove test code

17ff847

Change Coin to (de)serailize from and to string(CRO125)

9d38eb8

tomtau requested review from calvinaco and devashishdxt July 1, 2019 13:38

tomtau suggested changes Jul 2, 2019

View reviewed changes

tomtau closed this Jul 2, 2019

leejw51crypto deleted the cro125 branch July 17, 2019 02:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change Coin to (de)serailize from and to string(CRO-125)#168

Change Coin to (de)serailize from and to string(CRO-125)#168
leejw51crypto wants to merge 31 commits intocrypto-com:masterfrom
leejw51crypto:cro125

leejw51crypto commented Jun 28, 2019

Uh oh!

tomtau left a comment

Uh oh!

tomtau Jul 2, 2019

Uh oh!

tomtau Jul 2, 2019

Uh oh!

tomtau Jul 2, 2019

Uh oh!

tomtau Jul 2, 2019

Uh oh!

tomtau Jul 2, 2019

Uh oh!

tomtau Jul 2, 2019

Uh oh!

tomtau Jul 2, 2019

Uh oh!

leejw51crypto Jul 2, 2019 •

edited

Loading

Uh oh!

calvinaco commented Jul 2, 2019

Uh oh!

tomtau commented Jul 2, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

leejw51crypto commented Jun 28, 2019

Uh oh!

tomtau left a comment

Choose a reason for hiding this comment

Uh oh!

tomtau Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

tomtau Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

tomtau Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

tomtau Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

tomtau Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

tomtau Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

tomtau Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

leejw51crypto Jul 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

calvinaco commented Jul 2, 2019

Uh oh!

tomtau commented Jul 2, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

leejw51crypto Jul 2, 2019 •

edited

Loading