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

Implementation-of-flip-69 #123

Merged
merged 6 commits into from
Mar 15, 2023
Merged

Implementation-of-flip-69 #123

merged 6 commits into from
Mar 15, 2023

Conversation

satyamakgec
Copy link
Contributor

@satyamakgec satyamakgec commented Mar 1, 2023

Copy link
Member

@joshuahannan joshuahannan left a comment

Choose a reason for hiding this comment

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

I looked at the cadence code. Looks good! Can you also add implementations for the switchboard and token forwarders, and can you add a transfer transaction that accepts an address and public path as a parameter, gets the receiver reference from that path, then only deposits the tokens if the receiver.getSupportedVaultTypes() supports the type of the vault that the transaction is attempting to transfer? If it doesn't just deposit the vault back into the owner's vault

contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
contracts/FungibleToken.cdc Show resolved Hide resolved
contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
Copy link
Member

@joshuahannan joshuahannan left a comment

Choose a reason for hiding this comment

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

I'm good with it. Lets share this on discord to get some community input. Make sure to say that we want to upgrade this on testnet and mainnet soon

@satyamakgec satyamakgec marked this pull request as ready for review March 4, 2023 03:11
///
/// @return list of supported deposit vault types by the implementing resource.
///
pub fun getSupportedVaultTypes(): [Type] {
Copy link
Member

Choose a reason for hiding this comment

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

Thinking about this more, I would like the return type of this to be {Type: Bool}. You can still iterate through it with return.keys, and it makes checking a specific type much easier. Do you think you'll be able to make that change in time for the upgrade next week?

@joshuahannan
Copy link
Member

@satyamakgec You'll need to pull the latest from master before you push your new changes since it is out of date

@satyamakgec
Copy link
Contributor Author

@joshuahannan latest pull have taken and the return type also changed

@satyamakgec
Copy link
Contributor Author

May need to fix the Go tests to make the CI happy

Copy link
Member

@joshuahannan joshuahannan left a comment

Choose a reason for hiding this comment

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

I'll fix the Go tests right now

@joshuahannan joshuahannan merged commit 38066d1 into master Mar 15, 2023
@joshuahannan joshuahannan deleted the implementation-of-flip-69 branch March 15, 2023 14:45
@joshuahannan joshuahannan restored the implementation-of-flip-69 branch March 16, 2023 15:23
@joshuahannan joshuahannan deleted the implementation-of-flip-69 branch March 22, 2023 18:08
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.

3 participants