Zoksh

In contract GatewayRoute the merchants are able to take their share of fare by calling the settle() function. The other function refund() is used for refunding the users if the wrong purchase was made or for some reason where the user/customer didn’t like the product. But the merchant can pass his other address as the recipient to take all the money which allows the merchant to bypass Zoksh’s fees.

In contract GatewayRoute the merchants are able to take their share of fare by calling the settle() function. The other function refund() is used for refunding the users if the wrong purchase was made or for some reason where the user/customer didn’t like the product. But the merchant can pass his other address as the recipient to take all the money which allows the merchant to bypass Zoksh’s fees.

The GatewayFactory contract has the variable setFee initialized to 100. From the calculations in _settle() on L112 this would cause 100% of the amount to be deducted as fees, this way preventing the merchant from withdrawing any amounts and transferring all tokens to the gatewayAddr.Currently in the setFee() function the admin can pass in any value to adjust the fee and the merchant uses confirmFee() to accept the adjusted fee variable. If the fee set is much higher than expected, it can limit user interaction with the protocol.

In GatewayRoute, function settle() and refund() send is used for transferring the ether. Future changes or forks in Ethereum can result in higher gas fees than send provides. The .send() creates a hard dependency on 2300 gas units being appropriate now but not in the future. This may also affect functionality when a multi-sig wallet is used for such interactions.

For ease of searching and the use of monitoring tools the following events in GatewayRoute can be indexed, PaymentSettled(), PaymentRefunded(), ManagerUpdated(). In the emission of events as well, there is no need to cast an address to payable.In GatewayFactory, the RouteCreated event has a 4th event topic without a name.

For ease of searching and the use of monitoring tools the following events in GatewayRoute can be indexed, PaymentSettled(), PaymentRefunded(), ManagerUpdated(). In the emission of events as well, there is no need to cast an address to payable.In GatewayFactory, the RouteCreated event has a 4th event topic without a name.

Functions with an external visibility modifier are not called directly in the contract and are cheaper than public functions are. For functions that are not to be called internally, it is best practice to make them external for gas savings.In GatewayRoute, getBlocked(), getManager(), getGatewayAddr() and getAdminContract() can be declared external. In GatewayAddress, getGatewayAddress() and getAdminContract() can be declared external.

Functions with an external visibility modifier are not called directly in the contract and are cheaper than public functions are. For functions that are not to be called internally, it is best practice to make them external for gas savings.In GatewayRoute, getBlocked(), getManager(), getGatewayAddr() and getAdminContract() can be declared external. In GatewayAddress, getGatewayAddress() and getAdminContract() can be declared external.

The naming convention used in the codebase is not clear and reduces the readability of the entire codebase. The terms manager and merchant are used concurrently but they could refer to different entities.In GatewayFactory, the onlyManager() modifier checks that the caller is the same as the merchantAddr address provided in the Administrable (GatewayAccess) contract. Since the check is for the merchant and not the manager, it can be changed to onlyMerchant().
In GatewayRoute, the onlyMerchant() modifier checks that the caller is the same as the managerAddr address provided in the creation of the GatewayRoute contract. Since the check is for the manager and not the manager, it can be changed to onlyManager().

From GatewayFactory, the creation of a route can happen with createRoute() or deployRoute(). When routes are created using deployRoute() the merchant address and manager address are the same (msg.sender), but with createRoute() the manager can be any value passed in. The issue here is that the merchant no longer has the privileges to call onlyMerchant() functions if a separate manager is set when calling createRoute().

It is highly recommended to have above 95% of code functionality tested before going into production so as to catch bugs that could be introduced from user input as well as return values from function calls.

It is highly recommended to have above 95% of code functionality tested before going into production so as to catch bugs that could be introduced from user input as well as return values from function calls.

Some variables declared in the GatewayFactory and GatewayRoute contracts are unused and can be removed to reduce the contract size and clean up the codebase. In GatewayFactory, the owner variable declared on L8 is unused. In GatewayRoute, the blocked boolean variable is unused as well.The SafeMath library is also imported into the GatewayRoute contract on L3 but is never used.
merchantWhitelist() also has methods created around it but is unused as well. It would be expected that this whitelist is checked to make sure that merchant addresses are whitelisted before they are used in transactions.

Some variables declared in the GatewayFactory and GatewayRoute contracts are unused and can be removed to reduce the contract size and clean up the codebase. In GatewayFactory, the owner variable declared on L8 is unused. In GatewayRoute, the blocked boolean variable is unused as well.The SafeMath library is also imported into the GatewayRoute contract on L3 but is never used.
merchantWhitelist() also has methods created around it but is unused as well. It would be expected that this whitelist is checked to make sure that merchant addresses are whitelisted before they are used in transactions.