fix(arns): disallow negative token costs #156
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #156 +/- ##
===========================================
+ Coverage 90.43% 90.84% +0.41%
===========================================
Files 9 9
Lines 1986 1966 -20
===========================================
- Hits 1796 1786 -10
+ Misses 190 180 -10 ☔ View full report in Codecov by Sentry. |
dtfiedler
requested review from
atticusofsparta,
kunstmusik and
arielmelendez
and removed request for
atticusofsparta
| function utils.isValidBase64Url(url) | ||
| local isValidBase64Url = #url == 43 and string.match(url, "^[%w-_]+$") ~= nil | ||
|
|
||
| if not isValidBase64Url then | ||
| error("String pattern is invalid.") | ||
| end | ||
| return url | ||
| end |
There was a problem hiding this comment.
cruft from aos copy/pasta - we use more specific utils and this has been confused with those
| -- stub the process id as it is not required for this intent | ||
| local processId = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" | ||
| arns.assertValidBuyRecord(name, years, purchaseType, processId) |
There was a problem hiding this comment.
this part seems odd to be in the non-test code. without digging into the function I wonder why wouldn't assertValidBuyRecord take an optional processID instead
There was a problem hiding this comment.
I guess you're just getting the cost here, whereas the assertion function may be used in later on in the creation flow
| tokenCost = arns.calculatePermabuyFee(baseFee, demand.getDemandFactor()) | ||
| end | ||
| -- if token Cost is less than 0, throw an error | ||
| if tokenCost < 0 then | ||
| error("Invalid token cost for " .. intendedAction.intent) |
There was a problem hiding this comment.
Might be helpful for debugging if the bad value were in the error string too.
| @@ -1813,7 +1815,7 @@ addEventingHandler(ActionMap.ReassignName, utils.hasMatchingTag("Action", Action | |||
| local initiator = utils.formatAddress(msg.Tags.Initiator) | |||
| local checkAssertions = function() | |||
| assert(name and #name > 0, "Name is required") | |||
| assert(newProcessId and utils.isValidBase64Url(newProcessId), "Invalid Process-Id") | |||
| assert(utils.isValidAOAddress(newProcessId), "Process id must be a valid base64url.") | |||
There was a problem hiding this comment.
This util function is too permissive (e.g. eth names).
No description provided.