DemocracyEarth / ubi

Universal Basic Income token.
224 stars 39 forks source link

Packing balance and accruedSince in the same slot #111

Open greenlucid opened 2 years ago

greenlucid commented 2 years ago

instead of having:

mapping (address => uint256) private ubiBalance;
(...)
mapping(address => uint256) public accruedSince;

you can do this:

struct UbiAccount {
  uint224 balance;
  uint32 timestamp;
}

mapping(address => uint256) public ubiAccounts;

and update everything to support this new structure (cast uint256s to uint224s, etc)

Rationale

Every time you transfer, you need to write to both slots. Assuming best case (non-zero to non-zero), the updateBalance(...) that is called will incur:

(2900 + 2100) * 2 = 10_000

if you however have both the balance and accruedSince in the same slot, you rewrite on dirty and hot:

(2900 + 2100) + (100 + 100) = 5_200

all in all, this amounts to 4_800 gas saved.

greenlucid commented 2 years ago

The way streams / flows work looks confusing, I'm assuming it's not defined yet. If I were you, I'd store all info in a single struct:


struct UbiStream {
  address to;
  uint32 streamAmount;
}

struct UbiAccount {
  uint152 balance;
  uint32 accruedSince; // timestamp of last redeem
  uint32 accrualPerSecond; // if you're not a human, you can still be streamed ubi.
  bool isHuman; // allows to NOT require a poh check on transfer, which saves 2600 gas.
  // when the third party disables the isHuman and streams, it gets all the UBI the human didn't redeem.
  // even the UBI that is streamed to the disabled human (for technical reasons).
  uint32 streamedToOthers; // sum of ubi that this human is streaming to others.
  Stream[] streamsTo; // purposely not a mapping. allows a third party to disable the streams
  // on everyone that has been awarded an ubi stream from this party.
  // (should be slightly cheap since this array will be set to zero as it iterates, refunding gas)
}

note that im considering that every human is able to do multiple streams but I think that's a bad idea. I would just have each human either not streaming, or streaming everything to another human. That makes a few steps easier, and removes computation (that also results in less gas costs for everyone)

so, this is what id do. It's will be cheaper and way easier to interact with in the edge cases (revoking streams, awarding streams, updating human status of an account...)


struct UbiAccount {
  uint176 balance;
  uint32 accruedSince; // timestamp of last redeem.
  uint32 nStreams; // counts how many ubi streams are pointing at you.
  bool isHuman; // allows to NOT require a poh check on transfer, which saves 2600 gas.
  // when a third party disables the isHuman, it gets all the accrued UBI of one stream.
  // but the UBI that was streamed from other to this human will stay in the human.
  bool streamingToOther; // makes transfer cheaper, otherwise you'd have to SLOAD the "streamsTo"
  // address of this struct, that lives in another slot.
  // --- 2nd slot ---
  address streamsTo; // if streamingToOther, a whole human stream is awarded to this address.
  uint96 freespace;
}
JuanuMusic commented 2 years ago

For the first comment about the improvement: Any change like that would require a UIP so I would recommend you to fork UBI, create the implementation and then write a UIP with the change proposal.

For the second about delegations:

The way streams / flows work looks confusing, I'm assuming it's not defined yet. Yes, The way flows and streams work IS defined already, however, streams will not be released on UIP-1, only flows which is MUCH simpler and cheaper in gas.

The UBI Delegation pattern is based on a "Plugin" system where the delegation contract should implement the IUBIDelegator interface. I encourage you to push a version of your own IUBIDelegator implementation with all your proposed implementations.