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

VDOC Launch for realises #116

Closed
wants to merge 15 commits into from

Conversation

RickyCook
Copy link

No description provided.

@RickyCook
Copy link
Author

... meant to do that in our own fork SORRY!!

@RickyCook RickyCook closed this Feb 23, 2018
@miohtama
Copy link
Contributor

No worries @RickyCook

Just to note that the approach you are doing the whitelisting is less elegant than using signatures in the transactions.

You are now polluting Ethereum state with the whitelist status. This does not need to be the case as you know at the time of the payment if the person is whitelisted or not (you have this information in your database).

@RickyCook
Copy link
Author

RickyCook commented Feb 27, 2018

yeah, I totally agree! I love the "stateless login" things similar to JWT. though, we made a decision that we wanted people to be able to pay direct to the contract without adding any transaction data, so the only way that we were able to do that is eschew all the customer ID stuff and tie them to eth addresses. it's less elegant, but much much simpler.

and if you're wondering, no the fallback function wasn't included in this PR; I raised it early for review purposes!

that being said, is there any reason why your fallback function just does a throw? I would've thought it'd be better to call buy and then have buy throw if it needed more information?

@miohtama
Copy link
Contributor

miohtama commented Feb 28, 2018

@RickyCook I feel you made a wrong choice.

  1. Paying direct to contract is very, very, bad as discussed here

#53

  1. You can implement a single click Pay button without gas price, gas limit and data field copy pasting using web3.js and e.g. MetaMask as shown in the screenshots here

https://tokenmarket.net/what-is/how-to-buy-into-a-token-sale-with-metamask-wallet/

"Without any transaction data" is against the core principles of smart contracts - by not addressing the fact that smart contracts have functions make the contract quite dumb in the end.

@saravana87
Copy link

@RickyCook how do you transfer eth to smart contract successfully without adding transact data.

Between, I tried to send ETH to crowdsale address using myethwallet.com by connecting meta mask but its still showing error and transaction failed.
https://ropsten.etherscan.io/tx/0x174a997a3b015dec1aed999910604061b45c89666d41f598b59acc41a54f8638

Any advice would be much appreciatable.

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.

3 participants