Reproduce:
edit L34 in session/menu.js
const g_BarterResourceSellQuantity = 100;
and set it to 49 for example.
Then the prices won't decrease when bartering.
Differential D381
Prevent a Barter exploit. fatherbushido on Apr 24 2017, 3:16 PM. Authored by
Details
Reproduce: const g_BarterResourceSellQuantity = 100; and set it to 49 for example. See the bug catch test.
Diff Detail
Event TimelineComment Actions Nice find. The patch is correct and complete (can't find similar exploits). I tried the following line when pressing F9 and typing for (let i=0; i<500; ++i) Engine.PostNetworkCommand("type": "barter", "sell": "food", "buy": "metal", "amount": 1); to buy 500 metal for 500 food without dropping food prices Also tried selling 0 amounts or negative amounts, but we have early returns in that function, so it's ok. Things I noticed out of scope of this diff:
Comment Actions Build is green Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! http://jw:8080/job/phabricator/887/ for more details. Comment Actions Build is green Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! http://jw:8080/job/phabricator/888/ for more details. Comment Actions Build is green Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! http://jw:8080/job/phabricator/889/ for more details. Comment Actions Update. Early return in the sim if someone modify its gui code. Don't add warn to handle unit test. Perhaps we could do a similar check in commands.txt (and warn at that place?).
Comment Actions In fact, thinking a bit to it, the exploit described in the ticket is just a poor man exploit :) the price does not increase, but it also does not increase for other players. IMO this quantity should not be a parameter in the gui, but be fixed by the simulation as it influences the simulation: could be an additional property in each json resource, with a null value indicating a resource not barterable as this was required some time ago in the forum. (The same for the factor 5 when Shift to avoid such exploit). But these may be too big changes for A22: we may have a temporary fix for it, just allowing 100 and 500, and defer a complete fix to A23. Comment Actions Yes :)
Yes! I think of that too but I didn't think to use json file.
ok, thanks for your help! Comment Actions Patch works as expected. Comment Actions
Ah yes I forgot the menu.js comment Comment Actions Build has FAILED Link to build: http://jw:8080/job/phabricator/1630/ |