[splintercell]

> msg.sender.send(p.lastPayment); // refund previous price of pixel

This code suffers from Recursive calling vulnerability (same bug which caused the DAO to be hacked).

You're sending the money BEFORE you're updating the balances.

There is a better way to write the payment code (and also wouldn't take you more than 5 minutes to write).

Here is one way to do it:

    function colorPixel(uint x, uint y, bytes3 color) payable onBoard(x, y) { 
      Pixel p = board[x][y];
      lastPayment = p.lastPayment;
      p.lastPayment = msg.value;
      if (msg.value > lastPayment) {
        if(!msg.sender.send(lastPayment)) throw; // refund previous price of pixel
        p.color = color;                // set the color
      } else {
        throw;
      }
    }

Another shorter way:

    function colorPixel(uint x, uint y, bytes3 color) payable onBoard(x, y) { 
      Pixel p = board[x][y];
      require(msg.value > p.lastPayment);
      if(!msg.sender.send(p.lastPayment)) throw; // refund previous price of pixel
      p.color = color;                // set the color
      p.lastPayment = msg.value;      // set new cost to whatever the person sent in
    }

[scientaster]

Good analysis of the code, and nice correction. Agreed that you should always check your sends' return values, too! In this example you have to send more than the last guy, so I think a recursive call would still put in more ether than would be withdrawn. Still undesired though.

If it were fleshed out more it should probably return the ether to the last depositor instead of the purchaser.

Thanks for the feedback!; I'm tempted to turn this into an example Dapp later today.