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

bity.com crypto off-ramping integration #164

Merged
merged 21 commits into from
Jul 5, 2019
Merged

Conversation

TimDaub
Copy link
Collaborator

@TimDaub TimDaub commented Jun 21, 2019

Fixes #107

Checkout
Screen Shot 2019-06-27 at 18 09 28

Receipt
screen_shot_2019-06-27_at_16 45 44

Order Status
Screen Shot 2019-06-27 at 18 08 11

Todos:

  • Change all bootstrap usage to rimble-ui
  • Make dialogue show button and not prompt when opening Exchange.js
  • Make user confirm transaction to bity
  • Do proper validation of inputs to avoid user errors
  • Remove unnecessarily included contract files again
  • Make bity integration i18n ready
  • Remove Exchange.js bity integration left-overs
  • Cursor on IBAN form resets when changing text in the middle
  • Rename Cashout.js to e.g. Bity.js
  • Get minimum possible order amount through API
  • Store bity order in local storage and allow user to check it's history and status
    • Get correct block timestamp for bity order (it's mainnet not Plasma chain)
  • Resolve all TODOs in the code
  • Propagate all errors to the user
  • Custom <Loader> time? Or Serviceworker that transfers in the background?
  • Rebase against current master
  • Give annual amount indicator or error (maybe by quering list orders)
  • It's not clear whether test bity.com order went through (e.g. bank account name is missing) (it went through)

Problems

I've created issues for problems that I deemed were not solveable in the scope of this PR:

@TimDaub TimDaub force-pushed the feat/bity-integration branch 2 times, most recently from 28d13d8 to a75764c Compare June 25, 2019 11:21
@TimDaub TimDaub force-pushed the feat/bity-integration branch 4 times, most recently from 681c464 to 48b5e64 Compare July 1, 2019 12:23
"GB"
];

let MIN_AMOUNT_DOLLARS = 15;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please note that this value (15) gets overwritten when componentDidMount is called.

constructor(props) {
super(props);

const pk = localStorage.getItem("metaPrivateKey");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Really unhappy with this. But it's currently the way burner-modules are supposed to be written :/

Copy link
Member

Choose a reason for hiding this comment

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

Can we have a quick call about this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@iamonuwa and I had a call. I connected him with @dmihal who's leading the burner-core integration.

return (
<div>
<Box mb={4}>
{/* TODO: How to put this into i18n without creating a mess?*/}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Still unclear how this can be done with i18n. I'd propose to just ignore it for now...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@iamonuwa I did a bit more research and came up with the following issue: #173

Copy link
Member

Choose a reason for hiding this comment

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

i18next is what was used in the initial implementation. That's what the internationalization is running on

Copy link
Member

Choose a reason for hiding this comment

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

I'll take a look at this and see if there is a better solution to it

@TimDaub TimDaub marked this pull request as ready for review July 1, 2019 12:30
@TimDaub TimDaub changed the title Feat/bity integration bity.com crypto off-ramping integration Jul 1, 2019
@art1c0 art1c0 self-requested a review July 3, 2019 08:58
Copy link

@art1c0 art1c0 left a comment

Choose a reason for hiding this comment

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

Testing off-ramping functionality on mainnet with metamask:

  • Try to exit ETH to Fiat ($25)

At the stage or pasting the IBAN there's a warning in the console:

index.js:1437 Warning: A component is changing an uncontrolled input of type text to be controlled. Input elements should not switch from uncontrolled to controlled (or vice versa). Decide between using a controlled or uncontrolled input element for the lifetime of the component. More info: https://fb.me/react-controlled-components
    in input (created by Context.Consumer)
    in StyledComponent (created by Input)
    in Input (at Bity.js:378)...

Then after performing the exchange and metamask confirmation, got this:

Uncaught (in promise) Error: Returned error: Error: Error: [ethjs-rpc] rpc error with payload {"id":5958094193946,"jsonrpc":"2.0","params":["0xf869328477359400809442e17e25f885a0be481867bdc8aab458c442996988012baa5ac39f67d68026a0d9800e725290a7aeae77538a8501de877a8eb4c4c075b6502ba597e30cc6a5bba005a2c1138ebb93fdbc331ee5c73290f83ad5fd9d80a0353ec3a9d7ba12da214c"],"method":"eth_sendRawTransaction"} {"code":-32000,"message":"intrinsic gas too low"}

Burner's interface stalled with this picture:
Screen Shot 2019-07-04 at 12 17 47

and there's an error in metamask as well:
Screen Shot 2019-07-04 at 12 18 16

Money not lost :)
However expected the transaction to be performed...

Later observed this in the console (maybe related):

Error: Returned values aren't valid, did it run Out of Gas?
    at ABICoder.push../node_modules/web3-eth-abi/src/index.js.ABICoder.decodeParameters (index.js:227)

@iamonuwa
Copy link
Member

iamonuwa commented Jul 4, 2019

Testing off-ramping functionality on mainnet with metamask:

  • Try to exit ETH to Fiat ($25)

At the stage or pasting the IBAN there's a warning in the console:

index.js:1437 Warning: A component is changing an uncontrolled input of type text to be controlled. Input elements should not switch from uncontrolled to controlled (or vice versa). Decide between using a controlled or uncontrolled input element for the lifetime of the component. More info: https://fb.me/react-controlled-components
    in input (created by Context.Consumer)
    in StyledComponent (created by Input)
    in Input (at Bity.js:378)...

Then after performing the exchange and metamask confirmation, got this:

Uncaught (in promise) Error: Returned error: Error: Error: [ethjs-rpc] rpc error with payload {"id":5958094193946,"jsonrpc":"2.0","params":["0xf869328477359400809442e17e25f885a0be481867bdc8aab458c442996988012baa5ac39f67d68026a0d9800e725290a7aeae77538a8501de877a8eb4c4c075b6502ba597e30cc6a5bba005a2c1138ebb93fdbc331ee5c73290f83ad5fd9d80a0353ec3a9d7ba12da214c"],"method":"eth_sendRawTransaction"} {"code":-32000,"message":"intrinsic gas too low"}

Burner's interface stalled with this picture:
Screen Shot 2019-07-04 at 12 17 47

and there's an error in metamask as well:
Screen Shot 2019-07-04 at 12 18 16

Money not lost :)
However expected the transaction to be performed...

Later observed this in the console (maybe related):

Error: Returned values aren't valid, did it run Out of Gas?
    at ABICoder.push../node_modules/web3-eth-abi/src/index.js.ABICoder.decodeParameters (index.js:227)

Which network are you testing with?

@art1c0
Copy link

art1c0 commented Jul 4, 2019

mainnet, as i noted in the first sentence :)

@iamonuwa
Copy link
Member

iamonuwa commented Jul 4, 2019

Okay,

Copy link

@art1c0 art1c0 left a comment

Choose a reason for hiding this comment

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

Successfully checked on the mainnet with and without metamask: order has been created and processed, view updated with basic details - good work @TimDaub !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate bity FIAT off-ramp
3 participants