Increase contract data key size back to 300 bytes

[earrietadev]

On Feb 20 this size was reduced from 300 bytes to 200 bytes (testnet) and it finally arrived in 200 bytes to mainnet with protocol 20. I was testing one of my protocols in futurenet and testnet without issues until Feb 12th when I started to prepare the code for the audit before launching on mainnet, at that moment I stopped testing on testnet during the whole period so I missed that change.

Yesterday with the final version of the audit I went to test again on testnet and I found out that none of my contract calls worked correctly, at first I thought it was related to protocol 21 but @leighmcculloch found that the reason was that one of the keys in the footprint was out of the current limits of the network (this key is 212 bytes).

Here is an example of a tx and its working simulation, and also the error response from the network after the simulation is sent: https://runkit.com/earrietadev/65e1626ece86f100080c2fb5 (the diagnostic says transaction footprint key exceeds network config limit)

And here is a similar tx made on Feb 12th: https://horizon-testnet.stellar.org/transactions/1b1c979ce7026886dd22d792dd2b4ef29dc46b788db379063253eeef9881666f

This is a blocker, so I would like to know if increasing this value to what it was before could be possible or at least if is something that is expected to grow in the future.

Related discussion on Discord:

• https://discord.com/channels/897514728459468821/1237998391582261349

[dmkozh]

Just to add some context here: 300 bytes has been picked as an arbitrary (small) limit. After getting some usage on futurenet/testnet, we've observed that keys stay below 200 bytes and decided to launch protocol 20 with a more conservative restriction of 200 bytes. Probably your transaction has slipped in the time between the evaluation and the actual update.

I'm not sure if a straight up 1.5x increase is feasible; the key size is a pretty performance-sensitive restriction. @SirTyson could maybe comment on this. We could make a vote for a small (say, 10% increase) happen, but I'm sure someone else will hit the increased limit eventually. It might be a good idea to try reducing the key size (e.g. use a Vec instead of struct when multiple fields are necessary) - small optimizations could shave off these 12 extra bytes. Also when the key is too large and can't be optimized, the right approach is to use a hash of all the data, which brings the any potential key size to a constant (just 32 bytes of custom payload). This has an additional benefit of reducing the storage costs, which will likely offset the slight CPU usage increase for hashing.

[SirTyson]

I personally don't think we should increase key size at all. Large keys definitely negatively impact performance and there's no real technical reason for having arbitrarily large keys. We can't really decrease this value once increased on pubnet because we'll break contracts, so I don't think we should increase it at all. If a key is above 200 bytes, it should just be hashed.

When you say you're currently blocked @earrietadev, can you describe your situation a little more? Is there a particular reason you need more than 200 bytes in the key instead of using a hash for the key?

[earrietadev]

Hi @SirTyson, I went with the suggestions by @dmkozh about using a vec instead of a struct and I also removed one of the values from the key so it fits within the network limits (it's still almost at the max size but in theory it should be ok from now on)

It was a blocker because if it wasn't possible to reduce the size so it fits then that meant I will need to go with separated contracts for each currency instead of a single contract that is multicurrencies, more contracts means more fees in rent and changing too much code so the app's front and backend code handle each contract.

Regarding the use of hashes, the reason I don't want to go with the hash route is because it makes hard to search entries in system/sites where they are decoded from XDR, for example in stellar.expert you can see the decoded entries of a contract but if everything is hashed is difficult to understand them, I went that route with sorobandomains (using hashes instead of saving full text) and some users mentioned that is hard to know what is what when they check the contract entries in those sites.

About this ticket, if you think the entry should stay at 200bytes then it's ok to close it

[dmkozh]

Regarding the use of hashes, the reason I don't want to go with the hash route is because it makes hard to search entries in system/sites where they are decoded from XDR, for example in stellar.expert you can see the decoded entries of a contract but if everything is hashed is difficult to understand them

That's a valid concern. I'm not sure if each and every contract entry has to be interpretable - in general, the internal data structure is an implementation detail. But I understand that for some contracts it might be beneficial to be able to interpret the keys. Maybe we could think of a different solution here. At least for the purpose of off-chain processing, it is possible to emit preimages as diagnostic events - we just need some standard that allows block explorers to implement the necessary logic just once. Not sure about footprint interpretability though - this seems pretty hard to achieve (though OTOH preflight could benefit from using the same preimage events, so maybe footprint itself doesn't have to be interpretable). So I think if this issue keeps coming up, we could try solving it via SEP and diagnostic events.

[leighmcculloch]

the internal data structure is an implementation detail

I don't disagree with this but, at the same time, the Soroban storage system is specifically designed to allow for rich interpretation about what is stored such that downstream systems can understand the values, because it is more scalable for downstream systems to process stored values than it is to execute contracts to read the values.

emit preimages as diagnostic events

+1. Are diagnostic events necessary or could this be actual events since the values outputted with an event should be able to be larger?

[dmkozh]

I don't see any value in hashing the preimages into ledger and diagnostic events seem completely safe here because it's trivial to verify that the preimages corresponds to the hash (and the hash itself is hashed into ledger)
We might want to have a separate category of diagnostic events though in order to reduce the costs for the systems that ingest data, though that generally requires some research on how the diagnostic events are being used currently.