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

Removed num_assets from NoteMetadata #170

Merged
merged 3 commits into from
Jan 24, 2024
Merged

Conversation

phklive
Copy link
Contributor

@phklive phklive commented Jan 23, 2024

Removed num_assets from NoteMetadata in the miden-node, this will enable the clean update of from the breaking changes made in the miden-base.

Linked to: 0xPolygonMiden/miden-base#414

Closes: #169

@phklive
Copy link
Contributor Author

phklive commented Jan 23, 2024

Hello @bobbinth, here are the changes to remove the num_assets from the miden-node.

I am having a problem when trying to use a non merged branch as a dependency (Here I have used your non merged PR branch for miden-lib and miden-objects to do my PR. I will update to main when your PR is merged).

This is the code that I have updated in the Cargo.toml of the miden-node to use your branch:

https://github.com/0xPolygonMiden/miden-node/pull/170/files#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542R7-R8

Here is the error that I am having. Would you have a solution?

SCR-20240123-iapq

I wasn't able to compile / run tests because of this error, hence it can not be completed yet.

@phklive phklive requested a review from bobbinth January 23, 2024 07:35
@bobbinth
Copy link
Contributor

Here is the error that I am having. Would you have a solution?

This is likely because not all miden base dependencies have been updated (i.e., some crates in miden node still point to next branch on miden base).

@phklive
Copy link
Contributor Author

phklive commented Jan 23, 2024

Here is the error that I am having. Would you have a solution?

This is likely because not all miden base dependencies have been updated (i.e., some crates in miden node still point to next branch on miden base).

Found it thank you, all tests pass.

Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you! Let's merge after the miden-base PR is merged (we'll need to revert changes to Cargo.toml files).

@phklive
Copy link
Contributor Author

phklive commented Jan 23, 2024

Looks good! Thank you! Let's merge after the miden-base PR is merged (we'll need to revert changes to Cargo.toml files).

Sure, please ping me when merged, I will update and merge when done.

@bobbinth bobbinth merged commit 261c960 into main Jan 24, 2024
4 checks passed
@bobbinth bobbinth deleted the phklive-remove-num-assets branch January 24, 2024 21:11
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.

Remove of num_assets from note metadata
2 participants