0

I'm working on a smart contract that allows users to subscribe to different Plans.

I have a nested mapping that returns a struct(Subscription). I wanted to create a function that returns all the subscriptions that exist in a specific plan(0,1,2..).

The function that I wrote doesn't return all structs it returns only the first one after that it returns only zeros.

totalSubscriptions increase when a new user creates a new subscription.

address[] allSubcribers contains all the addresses of subscribers.

currentId represents the id of the plan that a subscriber signed up for.

struct Subscription {
    address subscriber;
    uint start;
    uint nextPayment;
  }

mapping(address => mapping(uint => Subscription)) public subscriptions;

function getAllsubscriptions() external view returns (Subscription[] memory) {

    Subscription[] memory items = new Subscription[](totalLoans);
    for(uint256 i = 0; i < totalSubscriptions; i++) {
        uint256 currentId = i;
        LoanRequest storage currentItem = subscriptions[allSubcribers[currentId]][currentId];
        items[currentId] = currentItem;
        currentId += 1;
    }

    return items;
}

What do you think?

2
  • Please, can you share all your smart contract code? Commented Apr 23, 2022 at 9:44
  • 9/10 times using a For loop in smartContracts is the wrong way to do it. And here you are using two of them in a single function. It's not just that it doesn't work, but it's also expensive. Please share the rest of the contract too, so we might be able to modify the logic to better suit your needs.
    – Sky
    Commented Apr 23, 2022 at 19:04

2 Answers 2

1

To help you, I wrote a contract with functions that can return list of all addresses subscribed to a specific subscription plan (0,1,2 ....) + other functions like deleteUser(),subscribeUser() etc.

It's example of how you can do it all without for loops (you instead use mapping to remember "index")

This is a simple example where every user only has a single subscription, but it can be modified such that a single user has as many subscriptions as you wish. But I tried to keep it simple/redable so as to not overwhelm you.

Hope it helps.

// SPDX-License-Identifier: MIT
    pragma solidity ^0.8.10;
    
    contract SubscriptionContract {
    
     mapping(uint => Subscription) public subscriptionMap;
     mapping(address => UserData) public userMap; //Helps to know which user is subscribed to which service
    
    
    struct Subscription {
        string subscriptionName; //Netflix,Amazon etc
        address[] addresses;
    }
    
    struct UserData {
        uint subscriptionID; //Code number for Netflix,Amazon etc
        uint index; //Users location is array of addresses (So we can delete it without For loop)
    }
    
    function createSubscription(uint subscriptionID, string calldata name) public {
        subscriptionMap[subscriptionID].subscriptionName = name; //Netflic,Amazon etc
    
    }
    
    //User subscribes himself 
    function subscribeUser(uint subscriptionID) public {
    
        userMap[msg.sender].index = subscriptionMap[subscriptionID].addresses.length;
    
        subscriptionMap[subscriptionID].addresses.push(msg.sender);
        userMap[msg.sender].subscriptionID=subscriptionID;
        
    
    }
    
    
    //We delete specific user from subscription
    function deleteUser(address userAddress) public {
    
        //First we find his subscriptionID
        uint subscriptionID = userMap[userAddress].subscriptionID;
    
        //Next we find user's location in the array
        uint index = userMap[userAddress].index;
    
        //Now we delete the user from subscription (if it's there)
    
        //Deleting a member in array is done by replacing it with the last member of the array
        uint lastIndex= subscriptionMap[subscriptionID].addresses.length -1;
        subscriptionMap[subscriptionID].addresses[index] = subscriptionMap[subscriptionID].addresses[lastIndex]; //Replacment
        subscriptionMap[subscriptionID].addresses.pop(); //Removing value at lastIndex
    
    }
    
    
    function getAllSubscribers(uint subscriptionID) public view returns (address[] memory) {
    
        return subscriptionMap[subscriptionID].addresses;
    
    }
    
    }

EDIT: Considering you said the first answer did not suit your needs, I just tried to fix your code (What is a bit tricky as we don't have entire contract so I have to guess some stuff)

I think your issue was mixing unit currentIndex vs unit currentId, you seem to be using both instead only one of those variables.

Below is adapted version of your code.

// SPDX-License-Identifier: MIT
    pragma solidity ^0.8.10;
    
    contract AllSubscriptions {



struct Subscription {
    address subscriber;
    uint start;
    uint nextPayment;
  }


mapping(address => mapping(uint => Subscription)) public subscriptions;

uint totalSubscriptions; //Number of subscriptions

function addSubscription(uint subscriptionID, uint startDate) public {

    subscriptions[msg.sender][subscriptionID].start = startDate; //Added new subscription
    subscriptions[msg.sender][subscriptionID].subscriber = msg.sender; //Added new subscription
    totalSubscriptions +=1;
    }

function getAllsubscriptions() public view returns (Subscription[] memory) {

        Subscription[] memory items = new Subscription[](totalSubscriptions);

        //We asume subscriptionID's are incremental (from 0 -> totalSubscriptions)
        for(uint i = 0; i < totalSubscriptions; i++) {
            uint subscriptionID = i;
            // pointer 
            Subscription storage currentSubscription = subscriptions[msg.sender][subscriptionID];
            items[subscriptionID] = currentSubscription;
            subscriptionID += 1;
        }

        return items;
    }

    }

EDIT2: Second version of contract (Here for simplicity I assumed every user only has a single Subscription + subscriptionID cant be Zero)

// SPDX-License-Identifier: MIT
    pragma solidity ^0.8.10;
    
contract AllSubs {



struct Subscription {
    address subscriber;
    uint start;
    uint nextPayment;
  }


mapping(address => mapping(uint => Subscription)) public subscriptions;  //uint = 0 is a default value, so no subscription should have that number as its ID. 

uint totalSubscriptions; //Number of subscriptions

address[] allSubcribers; //Array contaning all subscribers
uint[] subscriptionList; //This way I know who has what subscriptionID

function addSubscription(uint subscriptionID, uint startDate) public {

    if (subscriptions[msg.sender][0].start == 0){
        allSubcribers.push(msg.sender); //Adding address to the list if this is its first subscription
        subscriptionList.push(subscriptionID);
    }

    subscriptions[msg.sender][subscriptionID].start = startDate; //Added new subscription startDate
    subscriptions[msg.sender][subscriptionID].subscriber = msg.sender; //Added new subscription
    totalSubscriptions +=1;

   

    }


function getAllsubscriptions() public view returns (Subscription[] memory) {

        Subscription[] memory items = new Subscription[](totalSubscriptions);

        //We asume subscriptionID's are incremental (from 0 -> totalSubscriptions)
        for(uint i = 0; i < allSubcribers.length; i++) {
            address subscriberAddress = allSubcribers[i];
            uint subscriptionID = subscriptionList[i];
            // pointer 
            Subscription storage currentSubscription = subscriptions[subscriberAddress][subscriptionID];
            items[i] = currentSubscription;
        }

        return items;
    }


    }
9
  • The concept that you proposed in your solution is very interesting but it is not suitable for the concept that I work on inside my smart contract, I have a nested struct that has more than 3 values in one struct and I want to keep all values in one struct. I edited my post. I want to return all the values inside the Subscription struct
    – David Jay
    Commented Apr 24, 2022 at 0:44
  • I added new solution
    – Sky
    Commented Apr 25, 2022 at 19:53
  • Your solution returns the same result that I already had. because msg.sender has only one subscription, I need to loop through all the user's addresses who have subscriptions
    – David Jay
    Commented Apr 26, 2022 at 1:02
  • 1
    Can a single user have a multitude of subscriptions?
    – Sky
    Commented Apr 27, 2022 at 8:44
  • 1
    Default visibility for variables is (internal) , if there is none declared. But I had left it like that only for readability. You can set whatever you like. (You can also remove totalSubscriptions array)
    – Sky
    Commented Apr 27, 2022 at 23:08
1

I feel that there is some problem with your code logic here. The value of totalSubscription and itemCount is the same, so there is no need to define itemCount. It would save you some gas too (as you would be getting rid of a loop).

As you mention totalSubscriptionstores the value of totalSubscriptions made by all the users on the platform. But, here in this code snippet, you are using it to fetch the subscriptions of a single user.

I think that the reason for getting zeroes after one value is that the msg.sender has only one subscription but you are looping more times than that.

2
  • It makes sense! How do you think should I loop through all the addresses of users who have subscriptions?
    – David Jay
    Commented Apr 26, 2022 at 0:59
  • If you just have to loop through all the subscriptions. You could just make a subsCount to keep count of number of all subs. Then make a mapping(uint => Subscription), map subscription as count increases. The address of the subscriber is already in the Subscription Commented Apr 27, 2022 at 14:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.