fix: A8 wrong network in VC #2904
Conversation
higherordertech
added
the
C0-breaking
| @@ -164,7 +167,11 @@ impl Assertion { | |||
| Self::VIP3MembershipCard(..) | | |||
| Self::WeirdoGhostGangHolder => vec![Web3Network::Ethereum], | |||
| // total tx over `networks` | |||
| Self::A8(network) => network.to_vec(), | |||
| Self::A8(networks) => networks | |||
There was a problem hiding this comment.
How do you think just add a filter to filter out types, which is NOT Litentry, Litmus, Polkadot, Kusama, Khala, or Ethereum ?
Then we don't need to create a new type A8SupportedNetwork.
There was a problem hiding this comment.
Does the client pass in the wrong networks? Shouldn't this be fixed at client side first?
But sure we could a second check logic here
And yes we can simply filter out unwanted types, or retain only desired ones
There was a problem hiding this comment.
@Kailai-Wang this is the leftover of refactoring of shifting network type from link_identity to request_vc. From IDHub point of view, they always wish the all supported networks. So they don't do any filtering at client side. We did the magic within get_eligible_identities().
There was a problem hiding this comment.
refactored to only filter instead creating new types.
There was a problem hiding this comment.
this is the leftover of refactoring of shifting network type from link_identity to request_vc. From IDHub point of view, they always wish the all supported networks. So they don't do any filtering at client side. We did the magic within get_eligible_identities().
I doubt that's related.
The main motivation here was to have a struct to represent exactly the accepted values for A8. Clients must specify one of them to get the right assertion.
higherordertech
force-pushed
the
fix-A8-wrong-network
branch
from
9e354b3
to
07d9e65
Compare
P-926 added enum A8SupportedNetwork to matching ts types, removed unsupported network by dataprovider for A8
!!!! This is a breaking change !!!
Context
1 This VC used achainable, and it only supported a subset of networks as below, instead all the network that we defined:
ethereum, polkadot, kusama, litmus, litentry, khala
Labels
Please apply following PR-related labels when appropriate:
C0-breaking: if your change could break the existing client, e.g. API change, critical logic changeC1-noteworthy: if your change is non-breaking, but is still worth noticing for the client, e.g. reference code improvementHow (Optional)
Testing Evidences
Please attach any relevant evidences if applicable
Before the fix:
After the fix:
