celo-org / celo-monorepo

Official repository for core projects comprising the Celo platform
https://celo.org
Apache License 2.0
684 stars 360 forks source link

Devs SBAT see updated SDK docs #761

Closed cmcewen closed 4 years ago

cmcewen commented 4 years ago

Expected Behavior

Devs see updated docs from hackathon feedback, and create new issues for larger chunks of work

nambrot commented 4 years ago

Feedback:

We managed to deploy our custom smart contract as follows. There were some inconsistencies between the documentation on signing transactions and what worked when deploying contracts.

...
const contract = (new kit.web3.eth.Contract(abi) as unknown) as ContractType;
// @ts-ignore
const transaction = contract.deploy({ arguments, data: bytecode });
// @ts-ignore
requestTxSig(kit, [{ from: address, tx: transaction }], {
  requestId,
  dappName,
  callback,
});
const dappkitResponse = await waitForSignedTxs(requestId);
const tx = dappkitResponse.rawTxs[0];
const result = await kit.web3.eth.sendSignedTransaction(tx);
...

Here are some of our notes on the Celo SDK:
There isn't any comprehensive documentation of all available functions and their signatures and behaviors for dappkit and contractkit 
There isn't any documentation listing all the core contracts that Celo provides
There isn't any documentation for creating and deploying smart contracts i.e. kit.web3.eth.Contract(...).deploy(...)
There are warnings when using the SDK which the documentation just tells us to ignore. While the yellow box can be hidden, the console output gets polluted. 
SECURITY RISK: when requesting for the account address, cancelling the request on the Celo Wallet app still yields the address and phone number
When requesting for the account address, regardless of the dApp name submitted, the Celo wallet always refers to the connecting app as "Savings Circle" 
Sending money as described in the ContractKit docs does not work and always results in an error. It is also different from how sending money is implemented in the dAppKit Docs.
The first line of the sample code in the ContractKit Examples say that we can do
const goldtoken = await kit.contract.getGoldToken()
but the correct code should be
const goldtoken = await kit.contracts.getGoldToken()
Typescript definitions do not match what is actually accepted by functions such as requestTxSig, forcing us to suppress typescript errors. For example, when signing a transaction for contract deployment kit.web3.eth.Contract(...).deploy(...), only from and tx are necessary when signing.
The sample code in the docs have missing imports
We couldnt find toTxResult even though it is used in the sample code, nor does it seem to be needed when sending transactions like this const result = await kit.web3.eth.sendSignedTransaction(tx);
The document doesn't explicitly indicate any support for etherium-based tools such as truffle and solidity, even though the savings circle source code implies support. This can be confusing for someone new to Celo, even if the docs say that Celo's blockchain reference is based on go-etherium
nambrot commented 4 years ago

Also created https://github.com/celo-org/celo-monorepo/issues/758 and https://github.com/celo-org/celo-monorepo/issues/757

mcortesi commented 4 years ago

Replaced by #841 #840 #839