claim_reward_balance_operation has no extension field

[youkaicountry]

claim_reward_balance will have to carry balances in multiple tokens, but there is no way to extend it as it is defined since it doesn't have an extensions field.

Possibilities I see:

1. Create a new operation with an extensions field, and a way to easily store an arbitrary number of tokens
2. Use the existing asset fields, since there are 3 of them

Both are messy in their own way. 1) is probably the more "correct" way, but it's going to be more work. 2) limits us to only 3 assets, and seems very unclean to me.

I am leaning towards 1), but are there other possibilities I am missing?

[theoreticalbts]

Actually we have another option:

• (3) Create a new operation with an extensions field, that only allows claiming a single token (instead of 3 tokens as the existing operation does).

This option is the "cleanest" IMHO. If a user wants to claim more than one reward, they may simply include multiple instances of the operation in a transaction.

Re-purposing the existing operation gives 90% of the benefit for 10% of the work, and we don't have two different operations around, one of which we then have to worry about how to deprecate.

So my preferences are (2), (3), (1) in that order.

[mvandeberg]

The merits of this operation depends on how many tokens we expect a user to claim, on average. I will define option (1) a bit more precisely. It has an extensions field and accepts a vector of tokens to claim.

Option (1), claiming a single token consumes a 16 byte account name, a 4 byte vector length, a 12 byte asset, and a 4 byte extension vector length.

Option (2), claiming a single token consumes a 16 byte account name, and 3x12 byte assets.

Option (3), claiming a single token consumes 16 bytes of an account name, a 12 byte asset, and a 4 byte extension vector length.

We can see the growth of overhead of these options as we increase the number of assets claimed.

Tokens	(1)	(2)	(3)
1	16 + 4 + 1*12 + 4 = 36	16 + 3*12 = 52	16 + 12 + 4 = 32
2	16 + 4 + 2*12 + 4 = 48	16 + 3*12 = 52	2*(16 + 12 + 4) = 64
3	16 + 4 + 3*12 + 4 = 60	16 + 3*12 = 52	3*(16 + 12 + 4) = 96
4	72	2*16 + 2*3*12 = 104	128
5	84	104	160
6	96	104	192
7	108	156	224
8	120	156	256
9	132	156	288
10	144	208	320

Option (1) wins at scale.

Option (2) is better only when we are claiming 3 assets.

Option (3) is better only when claiming one asset. However, because SBD and SP are guaranteed assets, it looses already.

I think option (1) is the best choice.

[youkaicountry]

I am leaning heavily towards efficiency. Creating and maintaining a new operation is not ideal, but I also think (1) is the best choice.

[theoreticalbts]

So what's different about this operation that we don't apply the same logic to, say, the transfer_operation? Sometimes a user might want to send to multiple recipients, why don't we create a new transfer_operation so that instead of:

struct transfer_operation
{
      account_name_type from;
      account_name_type to;
      asset             amount;
      string            memo;
}

we instead have

struct transfer
{
      account_name_type from;
      account_name_type to;
      asset             amount;
      string            memo;
}

struct transfer_operation
{
    vector<transfer> transfers;
}

In fact there's nothing special about transfer_operation, we could use this exact same argument to advocate for making this exact change to every single operation we have.

Why don't we do that? What's special about this operation that we only "vectorize" this operation, and don't vectorize any other operation?

Option (3) has a clear answer to this question: Nothing's special about this operation, so for consistency with our other operations, we won't vectorize it. Option (2) also has a clear answer: Vectorizing with a fixed length of 3 elements allows the new semantics to be backward compatible with the existing claim_reward_balance_operation.

Option (1) doesn't really have a clear answer.

Also, there's a related issue. How long does it take an operation to execute? For most ops, it is O(log(n)), basically meaning that there's a global limit on the number of database lookups / modifications an operation triggers [1]. But if you "vectorize" an operation then it could potentially be O(k*log(n)) where k is the number of internal "micro-ops". If you allow operations to execute loops, then it becomes harder to estimate resource usage by counting operations.

[1] Amortized in some cases, for example a market order that consumes multiple orders from the books could force arbitrarily many database lookups, but we could consider "accounting" the database lookups for the consumed orders as being "charged" to the transaction that created them, not the transaction that consumes them.

[theoreticalbts]

TLDR, I agree option (1) is more efficient on the wire, and that's an important consideration.

But it subtly changes / breaks the blockchain's ontology [1], which makes me reflexively want to reject it -- I might be willing to accept it, but we should at least think carefully about it for a while.

[1] What is the meaning of an operation? How do you perform iteration? These are fundamental design decisions about how the "Steem VM" is designed. Maybe the decisions that were made when Steem was created aren't optimal, but they're what we have to work with. I'm concerned that throwing out the existing answers to those questions and making up different answers on a case-by-case basis is going to result in a Frankenstein hodgepodge of weird exceptions and special cases, so we have to explain to developers "well usually you do things THIS way, but for THIS operation you do things THAT way."

[mvandeberg]

The difference is probabilistic. How many transfers would we expect to occur in the dynamic length transfer op? If the value is 2, then I would be all in favor of creating a batch transfer operation that supports one to many transfers. (If it is not one to many then you cannot amortize the savings from repeated account names in the from field) It is a bit of a bet with SMTs that we will have en expected vector length of 2 or more. If it ends up being 1.4, then maybe we're averaging a loss of 1 byte that could have been more efficient with option (3), but if we guess it will be 1 and it ends up being 2, then we're already at a loss of 16 bytes. At large scales, the cost for being wrong add up quickly.

I cannot say that with absolute certainty that I paid close enough attention to reward balances as I should have when implementing them. However, we likely would not have gone with option (1) because SMTs were not being considered at the time.

After having done this analysis I wish that we would have done something similar on other operations during design.

[youkaicountry]

I agree with basing the decision on probabilistic efficiency, and the first paragraph of the above post by @mvandeberg definitely puts words to my feelings on the matter.

Another thing that strikes me is that the existing claim_reward_balance_operation has fields for STEEM, SBD, and vests. It seems to me that since the operation is already set up to be able to deliver multiple types of assets as rewards, having a vector for SMTs is a natural extension of that idea.

[youkaicountry]

We will go with option (1), creating a new operation with an extensions field, and a vector of tokens.