Error: Stack too deep, try removing local variables

jwgcarlyle · July 2015

Hi

I'm getting this error. I would like help knowing how deep "too deep" is - i.e. is there a rule of thumb for how many local variables are allowed? I've been constructing a number of structs with references and mappings to other structs, up to 4 deep. e.g. categories[categoryID].nominees[nomineeID].user.externalUserID. I have not been trying to recompile as I go, so I now need to refactor across the contract.

There are a couple of references to the error on Google, but none definitive on when it will occur.

James

chriseth · July 2015

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

jwgcarlyle · 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;
	}
	
}

chriseth · 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.

jwgcarlyle · 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

chriseth · 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.

jwgcarlyle · 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?

jwgcarlyle · 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?

chriseth · 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.

Howdy, Stranger!

Quick Links

Categories

Error: Stack too deep, try removing local variables

Comments