Flow•5mo ago

joshua | Flow (2024-07-09)

I'm looking to add some pre and post conditions to the generic FT and NFT transactions that we include in Ledger and encourage other projects to use to make them safer. Anyone have any suggestions for what we can include besides what I have listed in the issues? https://github.com/onflow/flow-ft/issues/162 https://github.com/onflow/flow-nft/issues/222

17 Replies

Needle•5mo ago

I've created a thread for your message. Please continue any relevant discussion in this thread. You can rename this thread using /title <new title> If this is a technical question that others may benefit from, considering also asking it on Stackoverflow: https://stackoverflow.com/questions/ask?tags=onflow-cadence

joshuaOP•5mo ago

@Giovanni S

Unknown User•5mo ago

Message Not Public

joshuaOP•5mo ago

assertions too

Unknown User•5mo ago

Message Not Public

joshuaOP•5mo ago

mostly assertions right now actually

Unknown User•5mo ago

Message Not Public

joshuaOP•5mo ago

For this transaction specifically: https://github.com/onflow/flow-ft/blob/master/transactions/generic_transfer_with_address.cdc I am thinking of the scenario when a malicious contract updates their FTVaultData view and createEmptyVault functions to return vaults and information for FlowToken, so that if someone said they wanted to transfer that contract's vault the view would instead make them transfer FlowToken instead. How can we prevent that with assertions? maybe doing a getType and checking that the identifier contains the address and contract name? Is there a contains function for String? Or some other way to do that?

Unknown User•5mo ago

Message Not Public

Giovanni S•5mo ago

Oh shoot I meant to add those to the ft/nft repos 🤦‍♂️ sorry for missing that

Unknown User•5mo ago

Message Not Public

Giovanni S•5mo ago

Good callout on malicious contract 🤔 If there's flexibility around txn params, maybe we could instead take either a vault type or identifier and extract the contract name & address from that (ideally we could just do something like type.definingContract as! &{ViewResolver} but that's a new Cadence feature) Then from that param, include a pre-condition that the tempVault type/identifier matches the one provided

joshuaOP•5mo ago

I'm using this right now and it works. 😆

        // Get the string representation of the address without the 0x
        var addressString = contractAddress.toString()
        if addressString.length == 18 {
            addressString = addressString.slice(from: 2, upTo: 18)
        }
        let typeString: String = "A.".concat(addressString).concat(".").concat(contractName).concat(".Vault")
        let type = CompositeType(typeString)
        assert(
            type != nil,
            message: "Could not create a type out of the contract name and address!"
        )

        assert(
            self.tempVault.getType() == type!,
            message: "The Vault that was withdrawn to transfer is not the type that was requested!"
        )

        // Get the string representation of the address without the 0x
        var addressString = contractAddress.toString()
        if addressString.length == 18 {
            addressString = addressString.slice(from: 2, upTo: 18)
        }
        let typeString: String = "A.".concat(addressString).concat(".").concat(contractName).concat(".Vault")
        let type = CompositeType(typeString)
        assert(
            type != nil,
            message: "Could not create a type out of the contract name and address!"
        )

        assert(
            self.tempVault.getType() == type!,
            message: "The Vault that was withdrawn to transfer is not the type that was requested!"
        )

ugly though

Giovanni S•5mo ago

tbh this boilerplate could probably live in a utils contract I can see it being more widely used given the push toward generic txns. not sure where to put it though

Unknown User•5mo ago

Message Not Public

joshuaOP•5mo ago

that is true, though we've kind of already committed to this transaction at least for now. it is what we use for ledger and what we've shared around so far, but we can definitely change that if needed, just might take some time

Unknown User•5mo ago

Message Not Public

Gaming

Programming

joshua | Flow (2024-07-09)