!refactor: introduce SaveCommunityToken() and change AddCommunityToken()
#3798
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (12)
|
0x-r4bbit
added a commit
to status-im/status-desktop
that referenced
this pull request
Jul 25, 2023
As discussed in status-im/status-go#3798, there was a need to separate the logic in `AddCommunityToken()` into two API. This commit adjust the client to these changes such that: 1. After community token deployment tx was sent, we create a `CommunityToken` and add it to the database for tracking purposes 2. Once the tx is mined or dropped, we add the community token to the community description and publish it, or we update the deployment state in the database Needs: status-im/status-go#3798
jrainville
approved these changes
Jul 25, 2023
mprakhov
approved these changes
Jul 25, 2023
0x-r4bbit
force-pushed
the
refactor/add-community-token
branch
2 times, most recently
from
1bbbcab
to
f91fbe4
Compare
…oken()` **This is a breaking change!** Prior to this commit we had `AddCommunityToken(token *communities, croppedImage CroppedImage)` that we used to 1. add a `CommunityToken` to the user's database and 2. to create a `CommunityTokenMetadata` from it which is then added to the community's `CommunityDescription` and published to its members However, I've then discovered that we need to separate these two things, such that we can deploy a community token, then add it to the database only for tracking purposes, **then** add it to the community description (and propagate to members) once we know that the deploy tx indeed went through. To implement this, this commit introduces a new API `SaveCommunityToken(token *communities.CommunityToken, croppedImage CroppedImage)` which adds the token to the database only and doesn't touch the community description. The `AddCommunityToken` API is then changed that it's exclusively used for adding an already saved `CommunityToken` to the community description so it can be published to members. Hence, the signature is now `AddCommunityToken(communityID string, chainID int, address string)`, which makes this a breaking change. Clients that used `AddCommunityToken()` before now need to ensure that they first call `SaveCommunityToken()` as `AddCommunityToken()` will fail otherwise.
0x-r4bbit
force-pushed
the
refactor/add-community-token
branch
from
f91fbe4
to
0bd4c55
Compare
0x-r4bbit
added a commit
to status-im/status-desktop
that referenced
this pull request
Jul 26, 2023
As discussed in status-im/status-go#3798, there was a need to separate the logic in `AddCommunityToken()` into two API. This commit adjust the client to these changes such that: 1. After community token deployment tx was sent, we create a `CommunityToken` and add it to the database for tracking purposes 2. Once the tx is mined or dropped, we add the community token to the community description and publish it, or we update the deployment state in the database Needs: status-im/status-go#3798
0x-r4bbit
added a commit
to status-im/status-desktop
that referenced
this pull request
Jul 26, 2023
As discussed in status-im/status-go#3798, there was a need to separate the logic in `AddCommunityToken()` into two API. This commit adjust the client to these changes such that: 1. After community token deployment tx was sent, we create a `CommunityToken` and add it to the database for tracking purposes 2. Once the tx is mined or dropped, we add the community token to the community description and publish it, or we update the deployment state in the database Needs: status-im/status-go#3798
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a breaking change!
Prior to this commit we had
AddCommunityToken(token *communities, croppedImage CroppedImage)that we used toCommunityTokento the user's database andCommunityTokenMetadatafrom it which is then added to the community'sCommunityDescriptionand published to its membersHowever, I've then discovered that we need to separate these two things, such that we can deploy a community token, then add it to the database only for tracking purposes, then add it to the community description (and propagate to members) once we know that the deploy tx indeed went through.
To implement this, this commit introduces a new API
SaveCommunityToken(token *communities.CommunityToken, croppedImage CroppedImage)which adds the token to the database only and doesn't touch the community description.The
AddCommunityTokenAPI is then changed that it's exclusively used for adding an already savedCommunityTokento the community description so it can be published to members. Hence, the signature is nowAddCommunityToken(communityID string, chainID int, address string), which makes this a breaking change.Clients that used
AddCommunityToken()before now need to ensure that they first callSaveCommunityToken()asAddCommunityToken()will fail otherwise.