Error: Stack too deep, try removing local variables

Comments

• chriseth Member Posts: 170 ✭✭✭

  July 2015

  Could you create a small example contract that fails to compile and post it somewhere, please?

  0 · Share on FacebookShare on Twitter
• jwgcarlyle Member Posts: 29 ✭

  July 2015

  ok, here it is. The premise is that I have a reward contract, where a sponsor organisation can set up and pre-fund one or more reward categories, and participants can nominate each other for kind acts in different categories. When the category closes, the reward funds are distributed to each nomination recipient in that category, by the ratio of the total nomination tokens that they've received.
  
  Any and all suggestions for improving ANY aspect of this (from a code/language/structure p.o.v) gratefully received.
  
  I've stripped out all of the registration code etc.

  contract RewardContract {
  
  	struct User {
  		bytes32 userName;
  		address userAddress;
  		uint accountBalance;
  	}
  	
  	struct Category {
  		// categoryID is a string, which could be a hashtag
  		bytes16 categoryID;
  		bytes32 categoryName;
  		uint endTime;
  		uint rewardTotal;
  		uint nominationTotal;
  		uint numNominations;
  		uint numNominees;
  		// linked list of neighbouring categories; used for iteration
  		bytes16 nextCategoryID;
  		bytes16 previousCategoryID;
  		// mappings within a category
  		mapping (uint => Nomination) nominationsBySeq;
  		// map userIDs to sequence, and sequence to nominees
  		mapping (bytes16 => uint) nomineeSeqsByUserID;
  		mapping (uint => Nominee) nomineesBySeq;
  	}
  	
  	struct Nomination {
  		User nominator;
  		Nominee nominee;
  		uint nominatedAmount;
  		bytes32 nominationReason;
  	}
  	
  	struct Nominee {
  		User nomineeUser;
  		uint nomineeTotal;
  	}
  	
  	address owner;
  	uint numCategories;
  	mapping (address => User) public usersByAddress;
  	mapping (bytes16 => User) public usersByUserID;
  	mapping (bytes16 => Category) public categories;
  	bytes16 public headCategoryID;
  	bytes16 public tailCategoryID;
  
  	function nominate(bytes16 categoryID, bytes16 nomineeUserID, uint nominatedAmount, bytes32 nominationReason) public returns (bool) {
  		// lookup users involved
  		User nominator = usersByAddress[msg.sender];
  		User nomineeUser = usersByUserID[nomineeUserID];
  		// lookup category
  		Category c = categories[categoryID];
  		// check that both nominator and nominee have been registered, and category exists and still open
  		if (nominator.userName == "" || nominator.accountBalance < nominatedAmount || nomineeUser.userName == "" || c.categoryName == "" || now > c.endTime) {
  			return false;
  		}
  		// get a nominee
  		Nominee n;
  		// check if user has been nominated in this category before
  		uint nomineeSeq = c.nomineeSeqsByUserID[nomineeUserID];
  		if (nomineeSeq == 0) {
  			// first time nomination in this category
  			nomineeSeq = c.numNominees++;
  			c.nomineeSeqsByUserID[nomineeUserID] = nomineeSeq;
  			// now construct nominee
  			n.nomineeUser = nomineeUser;
  			n.nomineeTotal = 0;
  			c.nomineesBySeq[nomineeSeq] = n;
  		} else {
  			n = c.nomineesBySeq[nomineeSeq];
  		}
  		// construct the nomination
  		Nomination nomination = c.nominationsBySeq[c.numNominations++];
  		nomination.nominator = nominator;
  		nomination.nominee = n;
  		nomination.nominatedAmount = nominatedAmount;
  		nomination.nominationReason = nominationReason;
  		// update balances
  		n.nomineeUser.accountBalance =+ nominatedAmount;
  		n.nomineeTotal =+ nominatedAmount;
  		nominator.accountBalance =- nominatedAmount;
  		c.nominationTotal =+ nominatedAmount;
  		return true;
  	}
  	
  }
  

  0 · Share on FacebookShare on Twitter
• chriseth Member Posts: 170 ✭✭✭

  July 2015

  Depending on what you do, you can have around 16 local variables (including parameters and return parameters). The problem with this particular code is that it has a lot of storage references which actually take two stack slots (because of packed storage you also need the offset inside the slot), although they do not have to, I will try to fix that. What you could do now is to refactor that function into smaller functions.

  0 · Share on FacebookShare on Twitter
• jwgcarlyle Member Posts: 29 ✭

  July 2015

  Thanks, @chriseth. I suppose that I can refactor the nomination-construction etc. as a separate function, but then I have to pass in all of the parameters (so I'm not sure if that actually reduces the number of local variables). I will refactor various ways tonight.
  
  Just so I'm clear, if I have a function with 1 uint parameter "_in" and 1 uint return value "_out", and have a local uint variable declared inside the function, then that is 3 out of a max of 16. And if I have a reference to a variable held at the contract level (i.e. outside the function) then it needs a further 2, i.e. 5 out of 16.
  
  I still need to read up on storage references and packed storage and the 2 stack slots bit.
  
  James

  0 · Share on FacebookShare on Twitter
• chriseth Member Posts: 170 ✭✭✭

  July 2015

  That is right. The 16 is also not a strict limit, it can be smaller, it depends on what you do with the variables.

  0 · Share on FacebookShare on Twitter
• jwgcarlyle Member Posts: 29 ✭

  July 2015

  Just out of interest, is this a hard limit due to the way in which the compiler works, or the EVM, or is the limit going to creep up with optimisation over time?

  0 · Share on FacebookShare on Twitter
• jwgcarlyle Member Posts: 29 ✭

  July 2015

  Sorry, also just read https://github.com/ethereum/wiki/wiki/Solidity-Features#tightly-packed-storage - this makes sense - Can I assume that within the function, there are 16(?) slots of 32(?) bytes allowed, and each local variable is packed into slots as per the rules at that link, and each parameter and return variable is placed into separate slots without packing?

  0 · Share on FacebookShare on Twitter
• chriseth Member Posts: 170 ✭✭✭

  July 2015

  The limit is imposed by the fact that the EVM can only reach a specific depth into the stack and the fact that local variables are stored on the stack in Solidity.
  If this limit keeps on being a problem after the change I proposed above, it might be worthwhile considering moving some of the variables to memory.
  My own impression is that (again, after the change which will reduce the stack load to about half in your example) this limit should not be a real problem in reasonably written code.
  Please correct me if I am making wrong assumptions there.
  
  Concerning your second comment: This is about storage, not about the stack. Variables are not tightly packed on the stack, i.e. even a bool variable will take a full stack position.

  0 · Share on FacebookShare on Twitter