[Feature] Multi Offer

[WGB5445]

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe):

Issue Number: #125

[WGB5445]

增加自动转换数组结构

    public fun create<Offered: store>(account: &signer, offered: Offered, for: address, lock_period: u64) acquires Offers, Offer {
        let time_lock = Timestamp::now_seconds() + lock_period;
        //TODO should support multi Offer?
        let account_address = Signer::address_of(account);

        if(exists<Offers<Offered>>(account_address)){
            let offers = &mut borrow_global_mut<Offers<Offered>>(account_address).offers;
            Vector::push_back(offers, Offer<Offered> { offered, for, time_lock });
        }else {
            let offers = Vector::empty<Offer<Offered>>();
            if(exists<Offer<Offered>>(account_address)){
                Vector::push_back(&mut offers, move_from<Offer<Offered>>(account_address));
            };
            Vector::push_back(&mut offers, Offer<Offered> { offered, for, time_lock });
            move_to(account, Offers<Offered> { offers });
        }
    }

[baichuan3]

Offer需要考虑排重不？以及offers的长度限制

[WGB5445]

Offer需要考虑排重不？以及offers的长度限制

由于Offer将资源 Offered放入其中，所以每一个item都应该是单独的资源，不能算作重复项

offers可能会有长度问题，但是该资源为当前 Account创建并发放 Offer应该不会有攻击问题。

但是这里引入另一个问题，Offer需要Account有收回的权限，这样 Offer过多可以退出一些

[baichuan3]

这段应该在while循环里面把？

[pause125]

现在的写法没问题，但是确实可以合到循环里面去

[WGB5445]

现在的写法没问题，但是确实可以合到循环里面去

已经改了效果还行

[baichuan3]

如果没有offer的话，是不是直接返回空，不直接abort ？

[WGB5445]

这里需要兼容原有的函数，以防导致其他使用了 redeem 合约失效
在以前如果没有 Offer将会直接报错，函数接口也不好改动所以 abort处理应该是最好的选择了

[baichuan3]

redeem和redeem_v2的代码可以考虑合并

[WGB5445]

redeem和redeem_v2的代码可以考虑合并

这里还是有一些差别，刚才试验了一下不太好合并

[pause125]

现在的写法没问题，但是确实可以合到循环里面去

[pause125]

这里不考虑 for 接受多个 offer 吗？

[WGB5445]

这个主要是为了兼容 redeem 一次一取应该可以满足了，剩下的可以调用redeem_v2精准取出

[pause125]

address_of 这个接口应该要废弃了，没必要加这个 else 吧

[WGB5445]

address_of 这个接口应该要废弃了，没必要加这个 else 吧

其他合约如果有依赖，对方的合约可能会直接挂掉

[WGB5445]

address_of 这个接口应该要废弃了，没必要加这个 else 吧

其他合约如果有依赖，对方的合约可能会直接挂掉

[pause125]

接口可以保留着。但是这样默认返回 0 的 address, 可能是错误的，而且调用者很难发现。

[WGB5445]

接口可以保留着。但是这样默认返回 0 的 address, 可能是错误的，而且调用者很难发现。

有道理，改为没有<Offer>就 abort 这样应该没问题了

[jolestar]

接口可以保留着。但是这样默认返回 0 的 address, 可能是错误的，而且调用者很难发现。

那感觉返回最后一个 offer 的 address，应该就和原来的逻辑是一样的

[WGB5445]

接口可以保留着。但是这样默认返回 0 的 address, 可能是错误的，而且调用者很难发现。

那感觉返回最后一个 offer 的 address，应该就和原来的逻辑是一样的

如果合约在编写时候不谨慎可能会有问题，给地址发 Offer，但这期间如果再调用 create 给其他人发（利用同一个资源不能存多份的特性，预期是失败）插入了新的 Offer ，如果后面需要做一些其他事情可能会导致流程卡死

let for = address_of(offer_address)
assert!( for == sender , error );
let offered = redeem(sender, offer_address);
// do some thing

[jolestar]

这种用法那就是用户的责任了，除非他用 Offer 的接口检查是否发送过。

[WGB5445]

这种用法那就是用户的责任了，除非他用 Offer 的接口检查是否发送过。

好的，那我再恢复一下之前的部分写法

[WGB5445]

通过集成测试中的例子发现，原有的逻辑如果自动升级可能导致原有合约逻辑被破坏，如下测试中如果自动升级为 Offers 会导致 address 返回值与预期不符
感觉需要重新思考 Move 标准库升级的逻辑：
这种公开的方法如果有其他合约调用，但是返回值的结果与原有预期不同，假如该合约采用不可升级的模式，会导致该合约废掉，这不符合 stdlib 升级的本意
所以对于非必要 break 时 应该尽量采用 _v2 的方式扩展，保留_v1 的能力

script {
    use StarcoinFramework::Account;
    use StarcoinFramework::Offer;
    use StarcoinFramework::STC::STC;
    use StarcoinFramework::Signer;
    use StarcoinFramework::Token::Token;
    fun create_offer(account: signer) {
        let token = Account::withdraw<STC>(&account, 10000);
        Offer::create(&account, token, @bob, 5);
        // test Offer::exists_at
        assert!(Offer::exists_at<Token<STC>>(Signer::address_of(&account)), 1001);
        // test Offer::address_of
        assert!(Offer::address_of<Token<STC>>(Signer::address_of(&account)) == @bob, 1002);
    }
}

[jolestar]

unpack_Offer_info 中间 Offer 的 O 大写有点奇怪

[WGB5445]

fixed

[jolestar]

sender 任何时候都能 retake 自己的 offer 吗？感觉也不太对。会出现发了 offer 又立刻取消的骗局。

[jolestar]

另外原来没有 retake 方法，是不是不用区分 v1 和 v2 了？

[WGB5445]

sender 任何时候都能 retake 自己的 offer 吗？感觉也不太对。会出现发了 offer 又立刻取消的骗局。

这里设计初衷是考虑如果时间锁设置过大会导致无法 redeem，增加 retake 可能会减少这种情况

感觉 retake 解决超时 一个月 时间未领取比较合理？时间设置过大是发起者的失误，合约不解决这个失误

[jolestar]

sender 任何时候都能 retake 自己的 offer 吗？感觉也不太对。会出现发了 offer 又立刻取消的骗局。

这里设计初衷是考虑如果时间锁设置过大会导致无法 redeem，增加 retake 可能会减少这种情况

感觉 retake 解决超时 一个月 时间未领取比较合理？时间设置过大是发起者的失误，合约不解决这个失误

感觉应该解决的是对方如果一直没来领取，然后 Offer 一直不能释放的问题。或者就设置成超过 lock_time*2 的时间后 sender 可以 retake 自己的 offer。这样可以留 lock_time 的时间给 for 来领取。

[WGB5445]

sender 任何时候都能 retake 自己的 offer 吗？感觉也不太对。会出现发了 offer 又立刻取消的骗局。

这里设计初衷是考虑如果时间锁设置过大会导致无法 redeem，增加 retake 可能会减少这种情况
感觉 retake 解决超时 一个月 时间未领取比较合理？时间设置过大是发起者的失误，合约不解决这个失误

感觉应该解决的是对方如果一直没来领取，然后 Offer 一直不能释放的问题。或者就设置成超过 lock_time*2 的时间后 sender 可以 retake 自己的 offer。这样可以留 lock_time 的时间给 for 来领取。

有想过使用锁定的时间长度来控制是否可以 retake ，但是现有字段的用法是保存到期时间，实现这个锁定时间相关的东西需要改动的地方还不小，处理兼容性问题时会非常不优雅，不如这个版本先设定过期 3 个月，这个长度也比较合适

    /// A wrapper around value `offered` that can be claimed by the address stored in `for` when after lock time.
    struct Offer<Offered> has key, store { offered: Offered, for: address, time_lock: u64 }

[jolestar]

另外 Offer 是否应该提供 xx_entry 的方法，感觉也可以在斟酌下。因为不同的 Offer 的处理逻辑可能是不一样的。Offer 应该是个基础 module，在这个基础上封装 TokenOffer，NFTOffer 等，提供 entry 方法可能比较合适。

[WGB5445]

另外 Offer 是否应该提供 xx_entry 的方法，感觉也可以在斟酌下。因为不同的 Offer 的处理逻辑可能是不一样的。Offer 应该是个基础 module，在这个基础上封装 TokenOffer，NFTOffer 等，提供 entry 方法可能比较合适。

这个问题思考过，但take_offer带有 Collection2 功能所以想给Offer留一个底层entry的功能，续写了其他 entry.
从 Offer 合约作为基础库角度应该只提供redeem/retake 直接返回 Offered
原有的 Offer中存在的take_offer 带有 Collection2::put(&signer, Signer::address_of(&signer), offered);本次是否要废弃处理或者搁置

[jolestar]

另外 Offer 是否应该提供 xx_entry 的方法，感觉也可以在斟酌下。因为不同的 Offer 的处理逻辑可能是不一样的。Offer 应该是个基础 module，在这个基础上封装 TokenOffer，NFTOffer 等，提供 entry 方法可能比较合适。

这个问题思考过，但take_offer带有 Collection2 功能所以想给Offer留一个底层entry的功能，续写了其他 entry. 从 Offer 合约作为基础库角度应该只提供redeem/retake 直接返回 Offered 原有的 Offer中存在的take_offer 带有 Collection2::put(&signer, Signer::address_of(&signer), offered);本次是否要废弃处理或者搁置

我觉得可以废弃掉，不然用户存到 Collection2 里，自己也拿不出来，还得自己部署合约处理，反倒麻烦。