go-ethereum icon indicating copy to clipboard operation
go-ethereum copied to clipboard

cmd, core, eth: supply delta tracking

Open alextes opened this issue 1 year ago • 13 comments

By all means, ignore this PR for now and focus on the merge.

This PR is based on #24723 by @karalabe / @holiman , however that PR was getting stale, and https://ultrasound.money needs this code to keep track of various models that take the current ETH supply as an input. We've been running the fork in production for a while, we can't expect you busy gethians to prioritise it now. So here we are. I was planning on using a private fork, some things broke in the rebase, after putting in the effort to properly understand what was going on and fix things, I felt I might as well contribute back the code.

This PR adds the following on top of #24723

  1. rewrite a commit to fix a seemingly accidentally removed flag (DocRoot).
  2. rewrite a commit to use new flag fns, some larger shift around updating to urfave/cli/v2 I believe.
  3. fix various typos.
  4. update to new trie.New interface requesting an owner arg (please closely review this change, I'll mark it in the diff).
  5. rename many bits as proposed by @JustinDrake and @holiman.
  6. add a parentHash to the supply delta notification.

I'm not married to anything in this PR. Feel free to edit, request edits, propose to drop some of the renamed bits, all fine 👍 .

I don't normally write golang, but apart from 2. and 3. everything logically appears unchanged to my untrained eyes.

Many blessings for the merge core devs 🐼 🙏 ✨ !

🦇🔊

alextes avatar Sep 01 '22 21:09 alextes

triage discussion: let's try to get this PR into shape, and get it merged.

holiman avatar Mar 14 '23 13:03 holiman

Rebased against the latest master.

Added three commits to fix code which missed a migration to some new interface. It builds again.

I could work those last three in using rebase if desired, I tried, things got messy, and decided it's not worth my time right now.

alextes avatar Mar 15 '23 14:03 alextes

I (hastly) rebased this on master and added withdrawal tracking for the shanghai hardfork here: https://github.com/mariusvanderwijden/go-ethereum/tree/supply-delta @alextes could you rebase this and maybe add the withdrawal tracking changes as well? (please don't cherry-pick my commit, the messages are pretty bad ;))

MariusVanDerWijden avatar Apr 13 '23 03:04 MariusVanDerWijden

@MariusVanDerWijden ya, they looked a bit messy haha (committing merge conflict markers 😅 ?) I did see you add a newline which is 👌 . Code needs room to breathe 😌 .

Anyway, more on-topic, why calculate withdrawals, log them, but then not emit them? (they aren't in the subscribed notification). Do devs benefit from seeing the logged withdrawals? The latter is really the only case I can think of where this is working as intended 😄 . If yes, 👍 . Otherwise, happy to add them to the notification.

alextes avatar Apr 19 '23 11:04 alextes

Yeah that code was written 5 minutes before the shanghai hardfork to make sure everything looks okay wrt. issuance. The primary goal was to be able to see the withdrawal amount in the console. If you think its useful to expose that everywhere, please do!

MariusVanDerWijden avatar Apr 19 '23 12:04 MariusVanDerWijden

Congrats on getting it in, in that case 😅 .

I have no use for withdrawal notifications, we track withdrawals on the beacon side. I'd suggest leaving the additional functionality out then. Features with zero users are 🙅.

alextes avatar Apr 20 '23 09:04 alextes

@alextes just checking on this PR - is this something that is still important for your group? I can look into getting this in shape for merge if so.

lightclient avatar Jul 12 '23 15:07 lightclient

@lightclient yup, we still run a node based on this branch and to my knowledge anyone that wants to figure out exactly how much ETH is in existence needs this code.

alextes avatar Jul 12 '23 15:07 alextes

Just curious, how do you utilize this? Do you then have an additional diff where you expose this data over RPC? Or is this command all you need to stop maintaining a fork?

lightclient avatar Jul 12 '23 15:07 lightclient

@lightclient it already works as is. You can see an example in the original PR #24723. You add the --supplydelta flag so deltas are stored for every newly synced block. Then this method existing on the api object at all is enough for one to be able to consume a RPC subscription. These docs describe it. The original PR again has an example using netcat. Not the most obvious way of adding an rpc subscription but it is nice and terse 😄 .

alextes avatar Jul 14 '23 09:07 alextes

This is ready for review again, cc @karalabe @holiman.

I rebased against master and made the following tweaks:

  • added a test in blockchain_tests.go
  • renamed package from core/supplydelta to core/supply since there were more methods in there than just supply
  • updated the function signatures a bit for the provided methods in the package
  • combined the fixed rewards with the uncle rewards (asked Justin, he is okay with this), and it makes more sense this way to me
  • added new geth_* rpc namespace to register the method under -- since this is such a geth-specific method this seems to make sense

Manually verified the output against the ultrasound website for both the logs and subscription and the burn matches what is shown there. @alextes if you have the ability to take a look at this branch and make sure it works correctly from your perspective, it would be appreciated!

lightclient avatar Jul 28 '23 22:07 lightclient

Triage discussion: put on hold, we want to see if this whole feature can be replicated using the new tracer-features (live chain tracing) and new event-hooks. Then we wouldn't need to have this as a 'native' geth-command.

https://github.com/ethereum/go-ethereum/pull/27629

cc @s1na

holiman avatar Aug 01 '23 12:08 holiman

Leaving a note I did do another rebase of my own version of this fork for Dencun. Heard from @s1na this may well be the last time I need to 🙌 .

Sidenote: My fork diverged from this one as updated by lightclient. To avoid a DB migration I used a private one.

alextes avatar Mar 13 '24 11:03 alextes

Guess this is not needed any more with the supply delta on master now.

karalabe avatar Jun 03 '24 12:06 karalabe