Add api requirements and template plugin #410
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| const token = allTokens[tokenId] | ||
| return token.denominations[0].multiplier | ||
| } |
There was a problem hiding this comment.
Bug: Token Existence Validation Missing
The getCurrencyMultiplier and getContractAddresses functions don't check if a token exists in allTokens before attempting to access its properties (like denominations or networkLocation). This can lead to runtime errors if allTokens[tokenId] returns undefined.
Additional Locations (1)
There was a problem hiding this comment.
Not an issue since it's the core that provides the tokenId and so we can be sure that allTokens includes the tokenId
| const { currencyConfig } = wallet | ||
| const { allTokens } = currencyConfig | ||
| const multiplier = getCurrencyMultiplier(currencyInfo, allTokens, tokenId) | ||
| return div(nativeAmount, multiplier, multiplier.length) |
There was a problem hiding this comment.
Bug: Precision Error in Denomination Conversion
In nativeToDenomination, the div function's precision parameter is incorrectly set to multiplier.length. This uses the string length of the multiplier instead of its actual decimal precision, which can lead to inaccurate conversion results.
There was a problem hiding this comment.
multiplier is actually a good estimate for precision as the multiplier is a power of 10 and determines how many decimal places the denomination can have.
paullinator
force-pushed
the
paul/apiReq
branch
from
e8ec787 to
f5f87c0
Compare
paullinator
force-pushed
the
paul/apiReq
branch
3 times, most recently
from
a36e55d to
c3b08f6
Compare
paullinator
force-pushed
the
paul/apiReq
branch
3 times, most recently
from
2683df8 to
357c9d2
Compare
samholmes
left a comment
samholmes
left a comment
There was a problem hiding this comment.
I've update the PR as part of my review. I'm setting request changes to mean that I'm requesting you accept the changes I added.
| @@ -0,0 +1,145 @@ | |||
| # Edge Exchange Provider API Requirements | |||
There was a problem hiding this comment.
We should add a paragraph immediately after the document title that gives a short abstract about what this document is about with an outline. That'd be useful.
|
|
||
| ### 6. Reporting API | ||
|
|
||
| Provider must provide an authenticated reporting API that allows querying of all transactions created using Edge. The reporting API must provide at minimum the following information for each transaction. Actual values are only examples. Similar values that can be mapped to the below are sufficient. API must allow paginated queries with start date, end date, and number of values. |
There was a problem hiding this comment.
What does "number of values" mean here?
docs/API_REQUIREMENTS.md
Outdated
|
|
||
| ### 9. User Authentication | ||
|
|
||
| Provider API should allow the Edge application to authenticate a user via cryptographically random authKey. The authKey should be created by Edge and passed into any quoting or order execution endpoint. If the authKey does not exist on the Provider system, the Provider's API should allow for account creation by receiving KYC info via API |
There was a problem hiding this comment.
"by receiving optional KYC info"? Or, how can we say that KYC isn't necessarily required because it depends on jurisdictional requirements?
|
/rebase |
github-actions
bot
force-pushed
the
paul/apiReq
branch
from
be02ed4 to
33f9afe
Compare
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
noneNote
Introduces guidance and scaffolding for new exchange integrations and streamlines chain mapping.
API_REQUIREMENTS.md,CHAIN_MAPPING_SYNCHRONIZERS.md, andCREATING_AN_EXCHANGE_PLUGIN.mdoutlining provider specs, mapping sync, and plugin creationsrc/swap/central/template.tswith full quote flow, error handling, EVM handling, and spend constructionscripts/allSynchronizers.ts; all provider synchronizers now useutil/loadMappingFile.ts(getMappingFilePath/loadMappingFile) and expose a consistentNAMEnativeToDenomination,denominationToNative,getContractAddresses, andgetCurrencyMultiplier; importsdiv/mul; minor typingsedge-currency-accountbased,edge-currency-plugins, andtypescriptWritten by Cursor Bugbot for commit 33f9afe. This will update automatically on new commits. Configure here.