storage: move token type translation to go #533
Conversation
storage/client/client.go
Outdated
| @@ -18,6 +18,7 @@ import ( | |||
| oasisConfig "github.com/oasisprotocol/oasis-sdk/client-sdk/go/config" | |||
| sdkTypes "github.com/oasisprotocol/oasis-sdk/client-sdk/go/types" | |||
|
|
|||
| "github.com/oasisprotocol/nexus/analyzer/runtime/evm" | |||
There was a problem hiding this comment.
dang ol storage depending on analyzer
There was a problem hiding this comment.
🤷 you could move it to common/types.go? Although it looks OK enough to me as-is. The true storage type is int, really ... But it's nice to do the automatic cast into analyzer types, because the compiler will warn us(*) if we change the db input (= analyzer) but not the handling of the db output (= storage/client).
(*) in limited cases, Go's approach to enums is a joke, call me at 1 800 VITRIOL for more pointless Go rants
There was a problem hiding this comment.
ah yeah, would make sense to move it to common, now that it shows up in both storage and api
go has a lot of progressive ideas about preventing authors from locking down their own stuff, including via enums
There was a problem hiding this comment.
moved. also it's now TokenType instead of EVMTokenType
There was a problem hiding this comment.
correction: doesn't show up in api. just analyzer and storage
| func translateTokenType(tokenType evm.EVMTokenType) apiTypes.EvmTokenType { | ||
| switch tokenType { | ||
| case evm.EVMTokenTypeERC20: | ||
| return apiTypes.EvmTokenTypeERC20 |
There was a problem hiding this comment.
dang ol storage depending on api
There was a problem hiding this comment.
Ya ideally it shouldn't, or at least not this directly. Can you define a storage version of EvmTokenType in storage/client/types.go please? You'll see all of those are actually just aliases for API types 🥲 -- but at least it gives us a layer where we could introduce different storage types if needed. In the past, we used to use this more actively when txs were stored in the db not fully parsed.
There was a problem hiding this comment.
ugh now I think this is fine. it lets us have the list of enum values in the api spec for the price of one copy of the values
pro-wh
force-pushed
the
pro-wh/feature/ttt
branch
from
6c815e1 to
e4230a0
Compare
| func translateTokenType(tokenType evm.EVMTokenType) apiTypes.EvmTokenType { | ||
| switch tokenType { | ||
| case evm.EVMTokenTypeERC20: | ||
| return apiTypes.EvmTokenTypeERC20 |
There was a problem hiding this comment.
Ya ideally it shouldn't, or at least not this directly. Can you define a storage version of EvmTokenType in storage/client/types.go please? You'll see all of those are actually just aliases for API types 🥲 -- but at least it gives us a layer where we could introduce different storage types if needed. In the past, we used to use this more actively when txs were stored in the db not fully parsed.
storage/client/client.go
Outdated
| @@ -18,6 +18,7 @@ import ( | |||
| oasisConfig "github.com/oasisprotocol/oasis-sdk/client-sdk/go/config" | |||
| sdkTypes "github.com/oasisprotocol/oasis-sdk/client-sdk/go/types" | |||
|
|
|||
| "github.com/oasisprotocol/nexus/analyzer/runtime/evm" | |||
There was a problem hiding this comment.
🤷 you could move it to common/types.go? Although it looks OK enough to me as-is. The true storage type is int, really ... But it's nice to do the automatic cast into analyzer types, because the compiler will warn us(*) if we change the db input (= analyzer) but not the handling of the db output (= storage/client).
(*) in limited cases, Go's approach to enums is a joke, call me at 1 800 VITRIOL for more pointless Go rants
pro-wh
force-pushed
the
pro-wh/feature/holders12
branch
from
6754434 to
e4cb046
Compare
pro-wh
force-pushed
the
pro-wh/feature/ttt
branch
2 times, most recently
from
35d9ba7 to
7c9ac25
Compare
pro-wh
force-pushed
the
pro-wh/feature/holders12
branch
from
e4cb046 to
91cd6aa
Compare
pro-wh
force-pushed
the
pro-wh/feature/ttt
branch
from
7c9ac25 to
fd03b59
Compare
pro-wh
force-pushed
the
pro-wh/feature/holders12
branch
from
91cd6aa to
a8d53b2
Compare
pro-wh
force-pushed
the
pro-wh/feature/ttt
branch
2 times, most recently
from
3d83231 to
981c4f2
Compare
pro-wh
force-pushed
the
pro-wh/feature/ttt
branch
from
981c4f2 to
5b4fb91
Compare
did we want to move this?