add rhs/onchain resolvers #12
Conversation
…ssuer DID from the context instead.
Upgrade go-schema-processor. Remove CredentialStatusResolveOpt, get issuer DID from the context instead.
resolvers/onchain.go
Outdated
| // OnChainResolverConfig options for credential status verification | ||
| type OnChainResolverConfig struct { | ||
| EthClients map[core.ChainID]*ethclient.Client | ||
| StateContractAddr common.Address |
There was a problem hiding this comment.
Can we have map[stateContractAddr]*ethclinet.Client
| return new(big.Int).SetBytes(utils.SwapEndianness(bs)) | ||
| } | ||
|
|
||
| func stateContractHasID(ctx context.Context, stateAddr common.Address, ethClient *ethclient.Client, id *core.ID) (bool, error) { |
There was a problem hiding this comment.
It is very psycho function 💂♂️
Why do you use mutex here and have duplicate code to get id, but one getter uses Readmutex but another uses FullLockMutext
idsInStateContractLock.RLock()
ok := idsInStateContract[*id]
idsInStateContractLock.RUnlock()
if ok {
return ok, nil
}
idsInStateContractLock.Lock()
defer idsInStateContractLock.Unlock()
ok = idsInStateContract[*id]
if ok {
return ok, nil
}
There was a problem hiding this comment.
https://en.wikipedia.org/wiki/Readers–writer_lock
https://en.wikipedia.org/wiki/Double-checked_locking
@ilya-korotya If this pattern is not clear to you after reading this, please ping me. I'll try to describe it with reference to this code.
But in this case, the double-check pattern is a bad idea because we perform a long operation when getting data from the network. I'll rewrite it.
resolvers/onchain.go
Outdated
| func newChainIDFromString(in string) (core.ChainID, error) { | ||
| var chainID uint64 | ||
| var err error | ||
| if strings.HasPrefix(in, "0x") || |
There was a problem hiding this comment.
I would prefer not to rely on the implementation of the ParseUint function. We support a chainID either in the format of decimal or hex. I don't think it is a good idea to encourage users to set the chain ID as 0b10101 or 0o123.
| return newIntFromBytesLE(stateBytes), nil | ||
| } | ||
|
|
||
| func newIntFromBytesLE(bs []byte) *big.Int { |
There was a problem hiding this comment.
-1
For me, it has the advantage of readability. I understand the author's intentions where it is used. Instead, I should interpret the meaning of this 'one line' to understand what it does.
| return new(big.Int).SetBytes(utils.SwapEndianness(bs)) | ||
| } | ||
|
|
||
| func stateContractHasID(ctx context.Context, stateAddr common.Address, ethClient *ethclient.Client, id *core.ID) (bool, error) { |
There was a problem hiding this comment.
https://en.wikipedia.org/wiki/Readers–writer_lock
https://en.wikipedia.org/wiki/Double-checked_locking
@ilya-korotya If this pattern is not clear to you after reading this, please ping me. I'll try to describe it with reference to this code.
But in this case, the double-check pattern is a bad idea because we perform a long operation when getting data from the network. I'll rewrite it.
resolvers/onchain.go
Outdated
|
|
||
| func stateContractHasID(ctx context.Context, stateAddr common.Address, ethClient *ethclient.Client, id *core.ID) (bool, error) { | ||
|
|
||
| idsInStateContractLock.RLock() |
No description provided.