Skip to content

MX Contract Integration #312

Merged
portuu3 merged 15 commits into
humanprotocol:mainfrom
vladNed:feat/mx-contracts-integration
Mar 16, 2023
Merged

MX Contract Integration #312
portuu3 merged 15 commits into
humanprotocol:mainfrom
vladNed:feat/mx-contracts-integration

Conversation

@vladNed

@vladNed vladNed commented Mar 10, 2023

Copy link
Copy Markdown
Contributor

Description

The goal of this PR is to implement all the human protocol sol contract in rust with the official mx-sdk-rs package

Summary of changes

All the code that represents the contract code can be found in any contract src directory. The meta and wasm crates are separate crates that contain the framework for the mx-sdk-rs .

Implemented the following contracts:

  • Escrow
  • EscrowFactory
  • KVStore
  • RewardsPool
  • Reputation
  • Staking

I also implemented some of the mock contracts for proxy calls only used in testing purposes.

Possible bugs

I don't know how solidity works properly with upgrading contracts, but in most of the MX contracts upgrading some of the contract require re-sending the init parameters.

Some of the contracts, for instance the RewardsPool require the reward token to be set each time the contract is upgraded. I left it as it is in the solidity contract but I suggest to set it only if its empty and use a separate endpoint to reset the rewards token to another token if its required on upgrade. This will also give the possibility of paying out first the existing token amount.

Also in the RewardsPool contract, I observed that rewards records can be added with a 0 amount after the fees were deducted. I left it as it is on the original sol contract but I suggest that the records are added only if the amount after fees is greater than 0.

How test the changes

Testing the contracts was added to the README file but you can always run cargo test into the mx directory to run all contracts tests. Or on each contract run the tests from the test_*.rs files.

Related issues

Become the official HMT Elrond maintainer

Resolves #138

Operational checklist

  • All new functionality is covered by tests
  • Any related documentation has been changed or added

@CLAassistant

CLAassistant commented Mar 10, 2023

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@vercel

vercel Bot commented Mar 10, 2023

Copy link
Copy Markdown

@vladNed is attempting to deploy a commit to the HUMAN Protocol Team on Vercel.

A member of the Team first needs to authorize it.

@portuu3 portuu3 self-requested a review March 13, 2023 07:51
Comment thread packages/core/mx/contracts/escrow-factory/src/lib.rs
Comment thread packages/core/mx/contracts/escrow-factory/src/lib.rs Outdated
Comment thread packages/core/mx/contracts/escrow-factory/src/lib.rs Outdated
Comment thread packages/core/mx/contracts/escrow/src/lib.rs
Comment thread packages/core/mx/contracts/escrow/src/lib.rs
Comment thread packages/core/mx/contracts/kv-store/src/lib.rs
Comment thread packages/core/mx/contracts/reputation/src/constants.rs Outdated
Comment thread packages/core/mx/contracts/rewards-pool/src/lib.rs Outdated
Comment thread packages/core/mx/contracts/staking/src/lib.rs
@portuu3

portuu3 commented Mar 14, 2023

Copy link
Copy Markdown
Collaborator

I don't know how solidity works properly with upgrading contracts, but in most of the MX contracts upgrading some of the contract require re-sending the init parameters.
in solidity storage keeps being the same when the contract is upgraded, so you don't really need to init again

@portuu3

portuu3 commented Mar 14, 2023

Copy link
Copy Markdown
Collaborator

Some of the contracts, for instance the RewardsPool require the reward token to be set each time the contract is upgraded. I left it as it is in the solidity contract but I suggest to set it only if its empty and use a separate endpoint to reset the rewards token to another token if its required on upgrade. This will also give the possibility of paying out first the existing token amount.

Let's do this

@psorinionut psorinionut left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I left a few suggestions & recommendations regarding mostly the code structure/architecture, and tried not to intervene as much in the code from the business logic perspective. Bottom line is that you should always try to optimise the code to go through as few lines of code as possible, and also try to reduce the storage readings to a minimum. This last part is very important, as it directly impacts the performance of the contract and this could make a difference as more users will begin to use it.

Also, I recommend you to upgrade to the latest framework (I see that now you are using version 0.39.2).

Comment thread packages/core/mx/contracts/escrow/src/status.rs Outdated
Comment thread packages/core/mx/contracts/escrow/src/lib.rs
Comment thread packages/core/mx/contracts/escrow/src/lib.rs Outdated
Comment thread packages/core/mx/contracts/escrow/src/lib.rs
Comment thread packages/core/mx/contracts/escrow/src/lib.rs
Comment thread packages/core/mx/contracts/staking/src/lib.rs
Comment thread packages/core/mx/contracts/staking/src/lib.rs
Comment thread packages/core/mx/contracts/staking/src/lib.rs Outdated
Comment thread packages/core/mx/contracts/staking/src/lib.rs Outdated
Comment thread packages/core/mx/contracts/staking/src/lib.rs Outdated
Comment thread packages/core/mx/contracts/rewards-pool/src/lib.rs
Comment thread packages/core/mx/contracts/rewards-pool/src/lib.rs
Comment thread packages/core/mx/contracts/rewards-pool/tests/test_rewards_pool.rs
Comment thread packages/core/mx/contracts/rewards-pool/src/lib.rs Outdated
@vladNed vladNed requested a review from portuu3 March 14, 2023 14:47

@portuu3 portuu3 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fix comment 1% and you can remove bulk_paid from escrow contract

Comment thread packages/core/mx/contracts/reputation/src/constants.rs Outdated
@portuu3

portuu3 commented Mar 15, 2023

Copy link
Copy Markdown
Collaborator

@vladNed can you please add a gitflow action to run tests?

@vladNed vladNed requested a review from portuu3 March 15, 2023 17:01
@vercel

vercel Bot commented Mar 16, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
exchange-oracle ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 16, 2023 at 11:02AM (UTC)

@portuu3 portuu3 merged commit 54386c1 into humanprotocol:main Mar 16, 2023
@vercel vercel Bot temporarily deployed to Preview – exchange-oracle March 16, 2023 11:02 Inactive
@vladNed vladNed deleted the feat/mx-contracts-integration branch March 16, 2023 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Become the official HMT Elrond maintainer

5 participants