# @Jan AdvancedEscrow Feedback
###### tags: `feedback`
## Comment
Overall very good. Easy to follow and perfect for the lazy who just want to copypaste code. Also very good descriptions of concepts.

Most of the issues are grammar related, but there are a few network/package errors that need explaining. Also there are a couple of code changes required, but very small considering how many lines of code!

## Edits
>The smart contract will allow users to initiate escrows in once currency and the beneficiaries to specify ...

Unclear sentence. Consider: "The smart contract will allow users to initiate escrows in one currency, and for beneficiaries to specify..."

-----
>The tutorial project will live in an advanced-escrow/ folder.

Unclear sentence. Consider: "..will live in the ..."

-----
> The local development network, which we will call mandala, and the public test network, which we will call

Perhaps worth mentioning that you can add private keys too with `accounts:["key1","key2"]` as not everyone (ie me), have their seed phrase to hand. Given we cant rebind ETH addresses to substrate this may come in handy to some.

-----
> Let’s take a look at the network configurations:

If this is a list may be easier to read? If not, add some fullstops for legibility.

-----
> The AdvancedEscrow smart contract we will add in the following section will still leave some areas that could be improved.

Unclear sentence. Consider: "... contract, which we will add in the following section, will still ..."

----
>When two parties enter into an escrow agreement using the AdvancedEscrow smart contract, the party paying for the service first transfers the tokens from one of the predeployed ERC20 smart contracts into the escrow smart contract and then initiates the escrow within the smart contract.

Very long sentence. Consider splitting: "When two parties enter into an escrow agreement, using the AdvancedEscrow smart contract, the party paying for the service: first transfers the tokens from one of the predeployed ERC20 smart contracts into the escrow smart contract. The party then initiates the escrow within the smart contract. "

----
> Initiation of escrow requires the information about the address of the predeployed token smart contract from which they transferred the tokens into escrow as well as the beneficiary of escrow.

Overly verbose. Consider: "Initiation of escrow requires both the contract address of the token being escrowed, and the wallet address of the beneficiary of escrow."

----
> Upon initiation of the escrow, the smart contract exchanges the tokens coming into escrow for the AUSD and sets the deadline after which, the AUSD is released to the beneficiary.

Very long sentence. Consider splitting: "...coming into escrow for AUSD. Then it sets the deadline after which, AUSD is..."

---
> We are also allowing for the escrow to be completed before the deadline, with the ability for the initiating party to release the funds to the beneficiary manually.

Confusing tenses. Consider: "We also allow for the escrow to be completed before the deadline, ..."

---
> Hardhat has already created a smart contract within the contracts/ folder when we ran it’s setup.
`its` not `it's`

---
> Our smart contract will support one escrow at the time, but will allow reuse.

Ambiguous. Add detail what `one escrow at a time` means. Is this globally? or per initiator? Inflight? etc

---

struct Escrow {
       address initiator;
       address beneficiary;
       address ingressToken;
       address egressToken;
       uint256 AUSDvalue;
       uint256 deadline;
       bool completed;
   }

`AUSDvalue` is a typo. The rest of the code is expecting `AusdValue`

---
> The Escrow structure holds the following information:

Consider formatting it as a list, or adding fullstops for clarity.

---
> The event contains information about the current state of the latest escrow:

Consider formatting it as a list, or adding fullstops for clarity.

---
> initiator: Address of the account that initiated the escrow

Highlighting incorrect

---
> As it is a boolean value, the escrow can either be active or fulfilled

Unclear sentence. As a bool it can only be true or false. Consider: "As an escrow can only be active or fulfilled, this can be represented as by a boolean value."

---
> Your contracts/AdvancedEscrow.sol should look like this:

There is a difference between the previous snippets and the single blob here: `IDEX` and `ISchedule` are no longer `public`

---
> we can import it into our deploy script and set the txFeePerGas and storageByteDeplosit

Typo in `storageByteDeposit`

---
> We need to get the signer which will be used to deploy the smart contract, then we instatiate the smart contract within the contract factory and deploy it

Typo in `instantiate`

---
> we will log it’s address to the console:

`its` not `it's`

---
> With that, we are able to run the deploy script using yarn deploy-mandala or yarn deploy-mandala:pubDev.

Maybe we should disclaimer that all the warnings are expected for now: `@polkadot/util has multiple versions, ensure that there is only one installed.
Either remove and explicitly install matching versions or dedupe using your package manager.`

---

ProviderError: Error: transaction hash not found (txHash="0x29277e98fa4aa91afac91cacdafed25d7900ab456088ab7cf904d6dee4085442", code=UNKNOWN_ERROR, version=bodhi.js/providers/2.1.11)
    at HttpProvider.request (/home/timbo/workspace/acala/advanced-escrow/node_modules/hardhat/src/internal/core/providers/http.ts:49:19)
    at LocalAccountsProvider.request (/home/timbo/workspace/acala/advanced-escrow/node_modules/hardhat/src/internal/core/providers/accounts.ts:188:34)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at EthersProviderWrapper.send (/home/timbo/workspace/acala/advanced-escrow/node_modules/@nomiclabs/hardhat-ethers/src/internal/ethers-provider-wrapper.ts:13:20)
----- tried it once and error, second time passed

This was a huge pain whilst using the public mandala network. We should give a warning and recommendation to just retry it until we fix the issue on our side.

---
> so we need to import their addressed

Typo in `addresses`

---
> Since we mate the escrows public, we

Typo in `make`

---
> we can use the automatically generated getted, to get the information about the escrow we have just created and output it to the console:

Typo in `getter`

---
> Your scripts/userJourney.js should look like this:

The big blob of code is missing the line: `console.log("Waiting for automatic release of funds");`

---
> WARNING: As you might have noticed, we initiated the escrow using a tenth of the funds that we transferred to the smart contract. This is because the smart contract needs to have some free balance in order to be able to pay for the scheduled call.

the initial balance / 10 is pretty unreliable and would not work for me.

In the end i had to hard code the values to be like 5 and 10 ACA, for transferring to the SC.