celestiaorg / go-square

A library for encoding blobs into a 2D square of evenly sized chunks designed for sampling and reconstruction
Apache License 2.0
13 stars 16 forks source link

Pass app version to `CreateCommitment` and `NewMsgPayForBlobs` #37

Closed evan-forbes closed 3 months ago

evan-forbes commented 10 months ago

When creating PFBs, and specifically PFB commitments, we will need to pass the app version to provide enough context for the application to generate the correct commitment for that version.

rootulp commented 6 months ago

Is this still necessary? CreateCommitment moved to the go-square repo and now accepts a param for subtreeRootThreshold which can vary based on app version.

evan-forbes commented 6 months ago

that's a good question, I'm not sure!

it might be best to pass app version to avoid breaking the api for those functions in the future when things change. Either way, we should move this to that repo

evan-forbes commented 3 months ago

Blocked on authored blobs, as that is the next change that could require this, at least for creating blobTxs, but probably not when we actually call create commitment. We might want to pass the app version anyway to avoid API breaks in the future when we inevitably do need the app version.

evan-forbes commented 3 months ago

@cmwaters quick questions: should we consider this part of authored blobs? if so, can we add this to the tracking for authored blobs? 🙏

cmwaters commented 3 months ago

Is this still necessary? CreateCommitment moved to the go-square repo and now accepts a param for subtreeRootThreshold which can vary based on app version.

This is correct. CreateCommitment can now handle changes to the subtreeRootThreshold that was hardcoded before. There is nothing else that is currently hardcoded. I would also refrain from adding app version to the go-square repo. I would prefer if it were agnostic to it and celestia-app would define the parameters but perhaps I could be proven wrong.