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

Upgrade to near-sdk-rs 4.1 and test functionality #23

Closed
ailisp opened this issue Apr 28, 2023 · 11 comments · Fixed by #26
Closed

Upgrade to near-sdk-rs 4.1 and test functionality #23

ailisp opened this issue Apr 28, 2023 · 11 comments · Fixed by #26
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@ailisp
Copy link
Collaborator

ailisp commented Apr 28, 2023

Problem
Currently DevHub contract is using near-sdk-rs 3.x. Several API enhancements have been introduced in 4.0 and security updates will also only update for 4.x so we should update to use near-sdk-rs 4.1.

User Story

As a developer, I want to use near-sdk-rs 4.x APIs in develop DevHub contract. Particularly I would like to use require! macro and new cross contract API.

Acceptance Criteria

  • Contract build with dependency near-sdk-rs 4.x in Cargo.toml
  • All existing functionalities still work. Can be test by deploying a copy of the contract and try functionalities in the DevHub widgets to hit all Near.call and Near.view.
  • Replace several if ... panic to require!
  • Replace cross contract call into social-db contract with 4.x API, ensure the notification on near social still works
@ailisp ailisp added enhancement New feature or request good first issue Good for newcomers labels May 2, 2023
@gautamprikshit1
Copy link
Contributor

I would like to work on this one

@frol
Copy link
Collaborator

frol commented Jun 7, 2023

I had to revert #26 (see 8fc1fdd) due to #26 (comment).

@ori-near
Copy link
Collaborator

Hi @frol – Can you please provide an update on this ticket? What's the next step here? Do we need to reassign or change the status?

@frol
Copy link
Collaborator

frol commented Jun 28, 2023

It seems that @gautamprikshit1 did not have time to proceed with this issue, so I unassigned him, and if someone is ready to jump on it, learn more in #26 (comment)

@frol frol closed this as completed Jun 28, 2023
@frol frol reopened this Jun 28, 2023
@jaswinder6991
Copy link
Contributor

Hi @frol, what is pending in this issue atm?
The list of tasks mentioned here - #26 (comment)
and the migrations you have mentioned? (which might be just the Sponsorship token according to you guess?)

@frol
Copy link
Collaborator

frol commented Aug 9, 2023

@jaswinder6991 well, by this time it will be easier to start the PR from scratch (just manually port the changes) as rebasing will probably just make things unnecessarily complicated.

@frol
Copy link
Collaborator

frol commented Sep 5, 2023

With #46 merged, this issue is unblocked, though I think that instead of trying to rebase #26, it would be easier to re-apply the changes manually. cc @jaswinder6991

@PiVortex
Copy link
Contributor

PiVortex commented Oct 2, 2023

I'm going to take a look at this.

@PiVortex
Copy link
Contributor

PiVortex commented Oct 3, 2023

Please help with this
See src/respost.rs
The test is failing as the two are not equal in the assert_eq!
Which way should it be structured? Should I reformat to be the same as the expected?
image

@frol
Copy link
Collaborator

frol commented Oct 3, 2023

@PiVortex The expected value is correct, the "post" value has to be a JSON-encoded string, not a JSON object.

image

@frol
Copy link
Collaborator

frol commented Oct 9, 2023

Resolved in #63.

@frol frol closed this as completed Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants