Feature/farming interface for frontend

[HyunggyuJang]

to proceed with implementing farming function for the interface side (client),
we need deployed farming contract (just you can deploy with interface. you don’t need to make full contract. so that implementation would be much easier. when it comes to farming if we could stake/unstake tokens, it would be enough features. )

By @SamuraiT

This PR provides skeleton of farming contract for that.

[PierreOssun]

I don't think there is a reason to store None values.
Best solution would be to store Hashmap<(id: u32, rewarder: AccountId), ()>
So you can easily check if an rewarder exist
Or Hashmap<id: u32, rewarder: AccountId> and only store where is is a rewarder (it will be None if id is not existing)
Id can be the pool id that will be stored (so pool_id +1)

[HyunggyuJang]

Agree, I'll work on that using Hashmap as you suggested. For now, let me deploy it to Shibuya, and I'll reflect this change later.

[HyunggyuJang]

Fixed as of 8e2dd3a

[PierreOssun]

save to storage only if Some()

[HyunggyuJang]

It was to align with proper pool id, but as you pointed out above, I'll change it using Hashmap structure.

[HyunggyuJang]

Fixed as of 8e2dd3a

[PierreOssun]

You should allow reentrancy for this or it will not work:

        PSP22Ref::transfer_from_builder(&lp_token, Self::env().caller(), Self::env().account_id(), value, Vec::<u8>::new())
            .call_flags(CallFlags::default().set_allow_reentry(true))
            .fire()
            .unwrap()?;

[HyunggyuJang]

Ah, I missed that! Thanks a lot. Will fix it

[HyunggyuJang]

Think we don't need to allow reentrancy. Pair contract only calls itself rather than transfer_from caller. Hence, we shouldn't allow reentrancy actually in this case.

[PierreOssun]

Yes I need to update in Pair contract. But here it is still relevant

[HyunggyuJang]

So, from the pair contract, you will callback the caller. But for what?

[HyunggyuJang]

Since it is LP token, the pair contract is the implementation.

[HyunggyuJang]

Ah, not the farming contract itself, the caller for deposit might call back the operator part of before_received: https://github.com/Supercolony-net/openbrush-contracts/blob/dc666f08c4e3bec2bf6057bb083cbf5223a28063/contracts/src/traits/psp22/psp22.rs#L160. So we need to allow reentrancy, is it correct?

If it were the case, still think we don't need to allow reentrancy.

[PierreOssun]

It's working for Pair because transfer_from is overridden and there is no _do_safe_transfer_check

wasm-showcase-dapps/uniswap-v2/contracts/pair/lib.rs

Line 147 in 9aa96d2

fn _transfer_from_to(

But other PSP22 implementation might impl the default Internal Trait - in this one there is _do_safe_transfer_check that will do a callback and will panic if we didn't allow reentrancy with flags.

Now you can still keep it like this (as LP are Pair contract) but it will not be compatible with standard psp22

[HyunggyuJang]

Yes, exactly. For this function, I think we'd better not to allow reentrancy to suppress from using invalid LP tokens.

[HyunggyuJang]

A note for myself:

Ah, not the farming contract itself, the caller for deposit might call back the operator part of before_received: https://github.com/Supercolony-net/openbrush-contracts/blob/dc666f08c4e3bec2bf6057bb083cbf5223a28063/contracts/src/traits/psp22/psp22.rs#L160. So we need to allow reentrancy, is it correct?

Is generally correct, but is not what happening in this situation. For the foregoing issue, the to of transfer_from is the contract itself, hence, if the PSP22 implementation is default one, then _do_safe_transfer_check calls before_received with to as a callee.
Plus, reentrancy check happens even before checking whether the callee has specified method or not, hence from foregoing context, it will raise an error saying reentrancy is not allowed.