Allowing general storage hitchhiking

[cxkoda]

Currently, ERC721A stores startTimestamp efficiently with each TokenOwnership in storage. While this is convenient for tokenomics, for some projects it would be nice to use the respective 64 bits for something else. More flexibility could be achieved by splitting the data providing logic (currently block.timestamp) into an own internal virtual function that is called on each transfer and renaming TokenOwnership.startTimestamp to something more generic like TokenOwnership.extraData.

Before submitting a PR, I wanted to check if this is something that you would like to support or if this is beyond the scope of ERC721A.

[Vectorized]

I see your idea. It is something I have been considering for some time.

This will pose some difficulties though.

Unlike _addressData, the _packedOwnerships array is lazily initialized. Using storage hitchhiking directly on it will break some of the core optimizations.

[cxkoda]

Oh sorry, I should have been more explicit. I was only talking about lazy-initializable data, e.g. using the blocknumber, or parts of the blockhash instead of the timestamp.

So on transfers having something like

...
_packedOwnerships[startTokenId] =
    _addressToUint256(to) |
    (_extraData(from, to, previousExtraData) << BITPOS_START_TIMESTAMP) |
    (_boolToUint256(quantity == 1) << BITPOS_NEXT_INITIALIZED);
...

function _extraData(address from, address to, uint64 previousExtraData) internal view virtual returns (uint256) {
   return block.number;
}

My usecase is for generative art, where we want to generate and store a random seed on mint. Using this scheme, we would generate a proto-seed for each batch, and calculate the actual seed of each token later via something like keccak(proto, tokenId). I'd override the above function to generate something new on mint, and reuse the previous value on transfers.

[Vectorized]

Oh, then it's possible.

That will be a breaking change though.

Currently, i can't think of an neat way to do it that makes the library easily extensible without much overhead.

The struct has a startTimestamp variable, and I can't think of a way to rename that to something more general.

I'd suggest you to make a fork.

[cxkoda]

Yes this will absolutely be a breaking change because startTimestamp needs to be renamed. Maybe to something like extraData? It would be very similar to the aux for addressData in a sense (only lazily initialized).

I submitted a quick draft implementation in #323 to show you in more detail how I intended this. Also talking code is easier most times. IMO, it can be added pretty straightforwardly without any overhead in terms of gas (the report remained completely unchanged on my end) and only small overhead in terms of maintainability (2 extra functions).

But I understand that this might be a feature that you don't necessarily want to support/maintain upstream. In that case I'll gladly fork.

[Vectorized]

The code looks good and well thought out.

Just concerned with introducing a breaking change right now, as we have only recently done a major bump just a few weeks ago. Even if we decide to merge it, it may take quite some time as we don’t want to do major bumps too often (it will cause many others to require changes in their code, and the ERC721A call center will be busy).

I’ll leave the PR open for others to use it as reference.

[cxkoda]

Awesome! Then I'll clean it up a bit and add some more comments and tests in the next days.

Regarding release, I totally understand! Happy if it gets considered for the next major version.

[Vectorized]

I'm thinking if 24 bits of extraData is enough. That's 16777216 possible values.

24 bits could be sufficient randomness for your described use case, since it is being keccaked with the tokenId, but would there be a need for 32 bits or more?

[cxkoda]

Could still be ok for some cases - but it's definitely not ideal.

For some reason I only noticed just now that there are actually 88bits of space that can be used. So I guess you were also thinking in the direction to just keep the 64bit for the startTimestamp and add extraData in the remaining 24, right?

I think that would be a good compromise to add the feature while maintaining backwards compatibility for now. Then we can expand it to the full range later if we want. If you want, I can update the PR to cover this tomorrow.

[Vectorized]

Let’s start with using the last 24 bits first.

At least the whole word can be used.

Tbh, I’m reluctant to break compatibility with previous versions. Cuz some people might have built functionality around startTimestamp.

[cxkoda]

Jup, that can very well be. I'd be curious if we could get some numbers or feel for the impact of such a change.

But also if there are some people using building on top of this - I think the migration would be fairly simple. In the end it's just renaming a variable because the underlying behavior still defaults to the same.

I updated the PR as discussed.

[Vectorized]

Let's close this issue for now.
If there is a need to extend the functionality in the future, we will re-open.