tools: Set reward pool to min balance when configured with DevMode

[barnjamin]

trafficstars

Summary

Addresses https://github.com/algorand/go-algorand/issues/4642

Test Plan

Locally tested with sandbox and verified that rewards are not applied but more tests are probably warranted

[codecov[bot]]

Codecov Report

Merging #4643 (d4db850) into master (3f1f3e4) will increase coverage by 0.24%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4643      +/-   ##
==========================================
+ Coverage   54.39%   54.63%   +0.24%     
==========================================
  Files         403      403              
  Lines       51921    51932      +11     
==========================================
+ Hits        28240    28372     +132     
+ Misses      21300    21165     -135     
- Partials     2381     2395      +14     

Impacted Files	Coverage Δ
gen/walletData.go	0.00% <ø> (ø)
gen/generate.go	65.12% <100.00%> (+51.02%)	:arrow_up:
ledger/tracker.go	74.04% <0.00%> (-3.83%)	:arrow_down:
network/wsPeer.go	66.50% <0.00%> (-1.95%)	:arrow_down:
ledger/acctupdates.go	70.19% <0.00%> (+0.59%)	:arrow_up:
data/transactions/verify/txn.go	77.14% <0.00%> (+0.95%)	:arrow_up:
catchup/peerSelector.go	100.00% <0.00%> (+1.04%)	:arrow_up:
catchup/service.go	69.62% <0.00%> (+1.48%)	:arrow_up:
crypto/merkletrie/node.go	93.48% <0.00%> (+1.86%)	:arrow_up:
crypto/merkletrie/trie.go	68.61% <0.00%> (+2.18%)	:arrow_up:
... and 2 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[jannotti]

I like the idea. My hesitation is that devmode might be too broad. Is it true that if you're using devmode you definitely don't want rewards? A year ago, I would have said absolutely not, since it would be very reasonable to want to test your code with rewards coming in. Now? Yeah, maybe. It's not as though there are rewards on mainnet. (Do we have a rewards pool on testnet?) So I guess a dev barely cares if their code acts weird under rewards.

Anyway, I guess I would be 100% on board if this was controlled by an option in the relevant CLI.

[barnjamin]

I understand the point about devmode being a broad way to flag this off but I also can't come up a reason that someone running devmode would want rewards enabled since we've turned them off on MainNet.

I could see an argument for also making this change to the autogen'd genesis.json file with a !devmode private network config in the case that the dev needs things like real timestamps or block seed but also wants no rewards. I'm not sure the right way to handle that, though.

Maybe another flag in the Genesis struct? or some special value of rwd field?

[pbennett]

I talked to Ben about this the other day. There was a time where 'rewards' would've been on my mind, but I lost a legitimate 2 days or so trying to figure out why my system tests were randomly failing. I had hit a block threshold in tests where my balance checks (get balance X.. issue transactions.. txns + inner transactions should equal... balance Y) were failing. I'd remove some tests -it would work. It made no sense. I had other changes at the same time so I thought I had to have broken 'something'.. 'somewhere' but couldn't find it. I tore everything apart in various ways trying to track it down.

Finally.. I asked Ben if he knew of anyting weird re. devmode. He mentioned part. rewards and it was a head-smack moment. I've long since put part. rewards out of my mind - but I instantly knew it was the problem.

Since part. rewards are still part of the protocol, I'd be inclined to have this be an independent configuration parameter so it could be enabled if you wanted to simulate it (because they might start up again - but perhaps just for online accounts for eg). If it's too much of a short-term hassle, using devmode is probably sufficient and it can be changed later.

This definitely should be changed either way. These are the kinds of non-obvious problems that will cause problems for devs new to Algorand. I was extremely familiar w/ part. rewards and their issuance but even I completely forgot about them. Someone new to Algorand would be dumbfounded by this behavior - only present when using sandbox/etc.

[barnjamin]

Thinking about it more, does it make sense to key off proto version?

https://github.com/algorand/go-algorand/blob/345c99d1a3093aa0fcec10be082fa10616685298/gen/generate.go#L75

[barnjamin]

Added another bool flag to the gen.GenesisData struct to allow specification of the NoRewards condition.

I chose to name it as NoRewards so the default init will match the current behavior of enabled rewards.

I did not add any logic to the NetworkTemplate.Validate method, I'm not sure this would cause any templates to be invalid but if that isn't the case please lmk

[pbennett]

Added another bool flag to the gen.GenesisData struct to allow specification of the NoRewards condition.

So this would just be a new flag in the network template? Yeah, this would be great actually. I just have a network template as part of my docker container and it would be a trivial change. We just need to make sure this is well documented so users don't continue to run into it. Perhaps it becomes the default in the sandbox created template, but if manually created users could still leave rewards enabled if desired.

[jannotti]

I already tried to reply, but it seems to have been lost. Sorry if it appears twice.

I don't like the idea of "override" of the template in goal. Devmode set a dangerous precedent in that regard, but I can see why: You have a full template that makes sense on its own, and you just want to run it in devmode. It does feel orthogonal.

But this feels like just a parameter that belongs in the template, and not overridable late in the game at network create time.

I do lean a little bit toward making that configuration an amount, along the lines of @michaeldiamant 's comments, as I think it reflects reality more closely. But I don't know if it super important.

[barnjamin]

Account balances after manual test in sandbox with 1000 pays w/ same snd/rcv for 0 algos.

Please ignore starting amount, the rewards field is the affected account field.

With patch && Genesis.RewardsBalance: 0

{'address': 'ANOKRWQFPMMKBEUAG644X5HTFX7N5BZF4U4Y7WK2KI5P3LVSFIH6IG745U', 'amount': 1999999999000000, 'amount-without-pending-rewards': 1999999999000000, 'apps-local-state': [], 'apps-total-schema': {'num-byte-slice': 0, 'num-uint': 0}, 'assets': [], 'created-apps': [], 'created-assets': [], 'min-balance': 100000, 'participation': {'selection-participation-key': 'NnBCnLEA+1vKojEdDf6udtaZ1E+vtpaJ0Z+3AN2r3GU=', 'state-proof-key': 'w1x/vC+OBBZKRIMUdCbVTLzYlFp+LwbWRtH9wNe6NOiucWTPGD3x1P0Ov/hvQriJbuRxNqab8fMR8bltRDkaDA==', 'vote-first-valid': 0, 'vote-key-dilution': 10000, 'vote-last-valid': 30000, 'vote-participation-key': 'ChBK7o9o1TFp9LG6Gl+o9RAxTwo+yp2CvSk6NKf8JKE='}, 'pending-rewards': 0, 'reward-base': 0, 'rewards': 0, 'round': 1000, 'status': 'Online', 'total-apps-opted-in': 0, 'total-assets-opted-in': 0, 'total-created-apps': 0, 'total-created-assets': 0}

With patch && unset Genesis.RewardsBalance:

{'address': 'EOD66G2YPR3PMEIK5CKLIHCCPKCRSDUIVGQLSUTXANBQCRLCCJ5TROJ6FQ', 'amount': 4000096000103978, 'amount-without-pending-rewards': 4000096000103978, 'apps-local-state': [], 'apps-total-schema': {'num-byte-slice': 0, 'num-uint': 0}, 'assets': [], 'created-apps': [], 'created-assets': [], 'min-balance': 100000, 'participation': {'selection-participation-key': 'BgOu9ayxCazT7wdFYDpdDLMm6QGCp3YJ1kPA+8sZq0w=', 'state-proof-key': 'wvhQVxi6bGX4OOGzC9tjIGHDOq4QoSZdfqGsXrxktp+H9XQU5ejApUqwoxPyaqXHPPPdfocdTkBWiNag749HXA==', 'vote-first-valid': 0, 'vote-key-dilution': 10000, 'vote-last-valid': 30000, 'vote-participation-key': 'tOPSz3PJNn3x0BElI9PKpf0qjBhVOC1Sanz04cJvcY8='}, 'pending-rewards': 0, 'reward-base': 24, 'rewards': 96001103978, 'round': 1000, 'status': 'Online', 'total-apps-opted-in': 0, 'total-assets-opted-in': 0, 'total-created-apps': 0, 'total-created-assets': 0}

Without Patch:

{'address': 'BCPM6P7PJT5RRA7SVSVKLIT6YYBYSKTOSJAM5VE6IRNHBSEHZKGEMGBQMQ', 'amount': 4000096000103978, 'amount-without-pending-rewards': 4000096000103978, 'apps-local-state': [], 'apps-total-schema': {'num-byte-slice': 0, 'num-uint': 0}, 'assets': [], 'created-apps': [], 'created-assets': [], 'min-balance': 100000, 'participation': {'selection-participation-key': 'DRmwzJI+dBSLzTyZU23kzY2qEzeFMefFlGcykqiaMr4=', 'state-proof-key': 'R+cf9N1SnUyrbxMI3DluqdkqruICxMuVBGI7Q8Y4liGVSRDGrTS4iTDJ5ckphSbsIAwQ7+cyTUmeGXNRqyEK4Q==', 'vote-first-valid': 0, 'vote-key-dilution': 10000, 'vote-last-valid': 30000, 'vote-participation-key': 'Brmi2od5h7QZ6uogIBWvR6Fxh7fWGYe2iDrdyK7vFSs='}, 'pending-rewards': 0, 'reward-base': 24, 'rewards': 96001103978, 'round': 1000, 'status': 'Online', 'total-apps-opted-in': 0, 'total-assets-opted-in': 0, 'total-created-apps': 0, 'total-created-assets': 0}```