Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PackageVersion to ContractInstance #19224

Closed
wants to merge 1 commit into from

Conversation

remyhaemmerle-da
Copy link
Collaborator

No description provided.

@remyhaemmerle-da remyhaemmerle-da changed the base branch from main to main-2.x May 16, 2024 19:59
@remyhaemmerle-da remyhaemmerle-da changed the title Remy pkg version Add PackageVersion to ContractInstance May 16, 2024
@remyhaemmerle-da remyhaemmerle-da requested review from a team and tudor-da May 17, 2024 07:47
@@ -139,6 +141,7 @@ message Versioned {
message FatContractInstance {
bytes contract_id = 1;
string package_name = 9; // required *since dev*
repeated uint32 package_version = 10; // required *since dev*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we adapt the comments to say since 1.16?

@@ -2125,7 +2125,10 @@ private[lf] object SBuiltin {

private def extractContractInfo(
tmplId2TxVersion: TypeConName => TransactionVersion,
tmplId2PackageName: (TypeConName, TransactionVersion) => Option[PackageName],
tmplId2PackageName: (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tmplId2PackageName: (
tmplId2PackageNameVersion: (

machine.lookupContract(coid) {
case V.ContractInstance(packageNameVersion, srcTmplId, coinstArg) =>
packageNameVersion match {
case Some((pkgName, _)) =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is pkgVersion used?

Copy link
Contributor

@tudor-da tudor-da left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, although I couldn't tell immediately where the new PackageVersion is used

@remyhaemmerle-da remyhaemmerle-da marked this pull request as draft May 17, 2024 12:38
@remyhaemmerle-da
Copy link
Collaborator Author

We will not persist package version for 2.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants