looks like the method should return a Vec<Promise> of modified others , shouldn't it?
So that individual elements of others can be used to chain promise calls further.

And to have some kind description of what type a contract call, doing then_many, returns, if Promise ever becomes a Promise<T> .

How would chaining with a Vec look? For single promises, it makes sense but not sure how it works for a group.

Instead, I thought we just borrow them, leaving the ownership at the caller. This makes it possible to reuse p3 and p4 in the doc comment.

And to have some kind description of what type a contract call, doing then_many, returns, if Promise ever becomes a Promise .

then_many doesn't create a new promise, it just creates dependencies between existing promises.
So, it doesn't have a result that could be used to return from a smart contract.

In the doc comment, you can return either p3 or p4. But not both at the same time. And their type is unaffected by then_many.

Does that make sense @dj8yfo ? Do you have a suggestion on how to make it more intuitive?

Maybe I should also give some context on how I want to use it.

This is for a sharded FT design of ft_transfer_call, as outlined in this comment.

near/NEPs#614 (comment)

And this is how I use it in the reference implementation:

https://github.com/jakmeier/near-sdk-rs/blob/c35b2bc2340cb77d719f6efc85c204d250e1e4ec/near-contract-standards/src/sharded_fungible_token/core_impl.rs#L196-L226

@jakmeier The proposed interface for then_many is error-prone in my opinion.

In the case of then and and, the self and other are consumed and a modified other is returned. In your doc string example:

    /// ```no_run
    /// # use near_sdk::Promise;
    /// let p1 = Promise::new("bob_near".parse().unwrap()).create_account();
    /// let p2 = Promise::new("carol_near".parse().unwrap()).create_account();
    /// let p3 = Promise::new("dave_near".parse().unwrap()).create_account();
    /// let p4 = Promise::new("eva_near".parse().unwrap()).create_account();
    /// p1.then(p2).then_many(&[&p3, &p4]);
    /// ```

p1 and p2 are consumed, but p3 and p4 are left for potential misuse (e.g. chain again with something else. It is not C++ world and we'd love to save devs from potential footguns.

In your reference implementation you want to return one of the final promises (deposit) as the transaction output value, but you don't want and cannot return results of two promises (well, you may join them in a reducing promise). Your intent sounds like "make these calls concurrently and detached", and make this one specific call the main execution branch. We brainstormed it today and originally thought of .then_many(other: Promise, detached: Vec<Promise>) -> Promise (which will return the modified other, similarly to .then() implementation), but this interface looks odd to me, and while I was typing I came to this realization that we kind of need the following:

    /// ```no_run
    /// # use near_sdk::Promise;
    /// let p1 = Promise::new("bob_near".parse().unwrap()).create_account();
    /// let p2 = Promise::new("carol_near".parse().unwrap()).create_account();
    /// let p3 = Promise::new("dave_near".parse().unwrap()).create_account();
    /// let p4 = Promise::new("eva_near".parse().unwrap()).create_account();
    /// p1.then(p2).and_detached(vec![p3, p4]);
    /// ```

where and_detached will return p2 and consume p3 and p4, or for your use-case, where you want deposit to be returned:

        let deposit = on_transfer.then(deposit).and_detached(vec![refund]);

        // The final value returned should be how much was transferred, to be
        // in line with NEP-141.
        deposit.into()

or you can even chain it all into a single line:

on_transfer.then(deposit).and_detached(vec![refund]).into()

What do you think? cc @dj8yfo

Looking at it again, I think it won't be clear that refund will receive the output from on_transfer and not from deposit, so maybe this one is better:

p1.then(p2).then_detached(vec![p3, p4]);

on_transfer.then_detached(vec![refund]).then(deposit).into()

Your intent sounds like "make these calls concurrently and detached", and make this one specific call the main execution branch.

This might be nitpicking but I'm thinking of it much more as "send the same data output to both these receipts". In your proposed designs, it's not very intuitive, at least for me, that deposit and refund are guaranteed to have the exact same callback inputs. Which is crucial for security.

This is clear:

p1.then(p2).then_detached(vec![p3, p4]);

But this isn't:

on_transfer.then_detached(vec![refund]).then(deposit).into()

@dj8yfo @frol What would you think of an interface like this?

pub fn then_concurrent(self, others: Vec<Promise>) -> ThenConcurrent;

struct ThenConcurrent {
    inner: Vec<Promise>,
}

impl ThenConcurrent {
    fn join(self) -> PromiseOrValue<T> {
        // note: this syntax doesn't work, we needs to `and` them pairwise
        Promise::and(self.inner)
    }

    fn return_by_index<T>(self, index: usize) -> PromiseOrValue<T> {
        // note: might want to use `get` and return Option here to prevent out of boundary access panic
        self.inner[index].into()
    }
}

Then we can allow chaining or returning but also protect from accidental misuse.

// this returns the result of p3 as the outcome of the current execution
p1.then(p2).then_concurrent(vec![p3, p4]).return_by_index(0);

// this wraps `p3` and `p4` with `and`, then executes `p5`
p1.then(p2).then_concurrent(vec![p3, p4]).join().then(p5);

// maybe we also can add more flexible ways of joining
// this wraps `p3` and `p4` with `and`, then executes `p5`
p1.then(p2).then_concurrent(vec![p3, p4, p5]).join_some(&[true, true, false]).then(p5);

@jakmeier I like the direction of thought, especially the join() idea. I don't like that much the indexing idea, but that is not terrible. Yet, what if I want to run p3 and p4 concurrently, but return p2 as the result, so p3 & p4 are kind of refunds. It is still possible to achieve that with a low-level as_return() API, but I think it is not widely known, and not easy to discover (though we may highlight the use of it in the then_concurrent() doc-string example:

p1.then(p2).as_return().then_concurrent(vec![p3, p4]);

Instead of join_some we may also think about joining them before the then_concurrent call:

let p3 = Promise...;
let p4 = Promise...;
let p3_and_p4 = p3.and(p4); // p3 and p4 are consumed into this new chain, and p3_and_p4 holds a new Promise with a Joint subtype with p3 and p4
p1.then(p2).then_concurrent(vec![p3_and_p4, p5]).return_by_index(0).then(p6);

Yet, for this to work, then_concurrent would need to recursively go down into the promises and find the leafs (where after is set to None), and that might be not intuitive, which is why it is forbidden in then implementation:

let p1 = Promise...;
let p2 = Promise...;
let p3 = Promise...;
let p2_then_p3 = p2.then(p3);
p1.then(p2_then_p3);
// ^^^ this technically could work and build the chain of p1 -> p2 -> p3,
// but the current implementation of `then` forbids it as it might be 
// confusing as `p2.then(p3)` returns `p3` and so `p1.then(...)` is ambiguous
// whether the intent was to call `p3` after `p1` as well as after `p2` (so "joined"(p1 -> p2) -> p3) 
// or just p1 -> p2 -> p3. The order is the same, but the number of inputs to p3 will be different.

Also, I don't think there is a value in returning PromiseOrValue from return_by_index(), as my contract functions may just need to return a Promise.
I don't find return_by_index clear enough, so I tried to find an alternative with conserns in the comments:

• then_concurrent([]).take_by_index(0).then(p6)
• then_concurrent([]).join_by_index(0).then(p6) (there is technically nothing to "join")
• then_concurrent([]).join_one(0).then(p6)
• then_concurrent([]).join_first().then(p6) ("first" is ambiguous - first one that completes or first from the list before)
• then_concurrent([]).after(0).then(p6)
• then_concurrent([]).wait(0).then(p6) (we are not "waiting" in the execution flow)
• then_concurrent([]).then(0, p6) (ugly and does not solve the case when I don't want to then and just want to pick one of the promises as the return value)
• then_concurrent([]).next(0).then(p6)
• then_concurrent([]).next_is(0).then(p6)
• then_concurrent([]).chain_after(0).then(p6)
• then_concurrent([]).first().then(p6)
• then_concurrent([]).wait_first().then(p6)
• then_concurrent([]).detach_all_except(0).then(p6) (ugly name, but this is my runner-up by clarity)
• then_concurrent([]).attach(0).then(p6)
• then_concurrent([]).select(0).then(p6)

I still hope there is a better name, but I couldn't find it :-(

@frol These are some good inputs.

You are right! My use case can also be done with as_return.
Then maybe let's consider if we are overengineering it here. For most use cases, then_concurrent() + join() is probably enough, isn't it? And for extra flexibility, even if awkward in chaining, we could add to_vec() that just returns the full vector of promises back from a JoinConcurrent struct.

On the other hand, if we want to provide a more complete API around concurrent promises, I'm starting to think we should take inspiration of the Rust std::vec::Vec API design for methods on ThenConcurrent.

My current favorite approach is with just copying the design of split_off. Basically, we define fn split_off(this: &mut ThenConcurrent) -> ThenConcurrent and it can be useful like so:

// p1 -> (p2, p3, p4 , p5)
// (p4, p5) -> p6
p1.then_concurrent(vec![p2, p3, p4, p5]).split_off(2).then(p6)

Or more generally:

let mut fan_out: ThenConcurrent = p1.then_concurrent(vec![p2, p3, p4, p5]);
let p4_and_p5: ThenConcurrent  = fan_out.split_off(2);
// fan_out now contains only p2 and p3
fan_out.join()...
p4_and_p5.join()...

My original use case would then look as follows.

// executes respecting on_transfer -> (refund, deposit)
// returns deposit as PromiseOrValue type
on_transfer.then_concurrent(vec![refund, deposit]).split_off(1).join().into()

Here I use that join on a ThenConcurrent with a single promise just returns that single promise. (the recursion base case)

Note that I also had to swap the order of refund and deposit for this to work in a chained syntax. But since the order in concurrent promises doesn't matter, we can always freely arrange those promises to best fit split_off able to divide into desired groups.

Also, I don't think there is a value in returning PromiseOrValue from return_by_index(), as my contract functions may just need to return a Promise.

I think you are right, I had misplaced concerns about returning it as a Promise when I wrote that last comment.

I still hope there is a better name, but I couldn't find it :-(

What do you think about split_off? It's not super clear IMO but at least fits the Rust world.

Also, how are you feeling about then_concurrent? I'm not entirely sold on that either.

I like then_concurrent. I like how we turned it all into join. I'm not a huge fan of split_off, but I think it is a reasonable trade-off. Will you have the capacity to implement it?

@frol Yes, I just pushed another commit that implements everything as discussed. PTAL when you have a moment.

I am concerted about it as Promise implements Drop:

which calls construct_recursively:

which calls construct_recursively on PromiseSingle, which calls the host functions to register a promise:

Thus, cloning self will result in 1+N calls to the self promise it seems like.

True, thanks for catching that!

I've fixed that by introducing reference counting on the after promise. This seems cleaner to me. It preserves how promises work prior to this PR, having exactly one Promise per index and only one drop call as a consequence.