onflow/flow-core-contracts

Refactor `FlowEpoch.getEpochMetadata` to avoid the dictionary-copy

SupunS opened this issue · 2 comments

SupunS commented

Issue To Be Solved

As mentioned in https://github.com/onflow/flow-core-contracts/pull/382/files/4aea8aa7057c43f7eaf4c8fdf1b08a50db7736b4#r1326279483, in the getEpochMetadata method, the reading of the metadata map from storage was changed from:

self.account.storage.borrow<{UInt64: EpochMetadata}>(from: self.metadataStoragePath)

To

self.account.storage.copy<{UInt64: EpochMetadata}>(from: self.metadataStoragePath)

Because, accessing the inner-object form the "borrow"ed map now returns a reference, where as the rest of the code-base has been written to work the dereferenced-object.

Suggest A Solution

However, this makes this operation very expensive, as it copies the entire map. To avoid this, we would need to either:

  • Support a way to get a de-referenced copy from a reference in Cadence (not supported yet)
  • Update the rest of the contract-code to work with the reference.

It would be good to go with the second option if possible, since that way we don't need to copy anything from the storage (not even the inner EpochMetadata object)

I think that the second one could work, but we would need to add a second method to get epoch metadata because we can't allow getting a reference to the metadata to be called by everyone. So there would be one private one that uses the reference and one public one that copies it

This was fixed in Janez' PR, so I think we can close this