Skip to content
This repository has been archived by the owner on May 19, 2023. It is now read-only.

feat: add new RNS contract #222

Merged
merged 2 commits into from
Jul 27, 2020
Merged

feat: add new RNS contract #222

merged 2 commits into from
Jul 27, 2020

Conversation

julianmrodri
Copy link
Contributor

@julianmrodri julianmrodri commented Jul 27, 2020

  • Bump version of RNS contract to v0.1.2
  • Use new RNS simple placement contract - Update Addresses.
  • Ignore Transfers handler when coming form the Marketplace Contract
  • Process transfers related to the Marketplace contract in the TokenSold handler.
  • Create registerTransfer function to avoid code duplication as Transfers are now generated in both handlers.

Works together with UI PR: rsksmart/rif-marketplace-ui#271
Setup and Deploy locally - Dev PR: rsksmart/rif-marketplace-dev#66

Copy link
Contributor

@AuHau AuHau left a comment

Choose a reason for hiding this comment

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

Just few small nitpicks.

A general question that I can't really assess is - is this backward compatible? Eq. for the events from the old contract will this still work?

src/services/rns/rns.processor.ts Outdated Show resolved Hide resolved
src/services/rns/rns.processor.ts Outdated Show resolved Hide resolved
src/services/rns/rns.processor.ts Outdated Show resolved Hide resolved
@julianmrodri
Copy link
Contributor Author

julianmrodri commented Jul 27, 2020

Adam,I think it is backward compatible. But anyway we DO NOT need to be backward compatible as this will start from scratch as an new contract on the Testnet, and then on Mainnet. Future versions Might need to be Compatible for sure.

Copy link
Contributor

@AuHau AuHau left a comment

Choose a reason for hiding this comment

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

LGTM, but you will have to rebase the last commit to commit-linting to pass, otherwise no merge 😅

Also if you will rename the RnsService you will make me happy 😇

@julianmrodri
Copy link
Contributor Author

Sure will fix the commit message and the rename.

@julianmrodri julianmrodri force-pushed the feature/newRNSContract branch 2 times, most recently from 269a7b4 to 6d935ce Compare July 27, 2020 16:43
@julianmrodri julianmrodri force-pushed the feature/newRNSContract branch from 6d935ce to 1f36b7c Compare July 27, 2020 16:48
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@julianmrodri julianmrodri requested a review from AuHau July 27, 2020 16:59
@julianmrodri julianmrodri merged commit 65084fd into master Jul 27, 2020
@delete-merged-branch delete-merged-branch bot deleted the feature/newRNSContract branch July 27, 2020 21:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants