-
Notifications
You must be signed in to change notification settings - Fork 3
Implement inner functionality for RFC-0001 #59
Implement inner functionality for RFC-0001 #59
Conversation
|
||
interface KeyManager { | ||
fun getSigner(publicJwk: Jwk): Signer | ||
fun getRustCoreKeyManager(): RustCoreKeyManager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this goes against our APID right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it does, but as far as I can tell it's necessary to make polymorphism work... we need to iterate on it because we want developers to be able to bring their own key manager's in which case this method would error-out. We'll come back to it later to add support, once we add WithForeign
in the UDL layer.
val purpose: String? | ||
val inputDescriptors: List<InputDescriptor> | ||
|
||
val rustCorePresentationDefinition: RustCorePresentationDefinition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have to do this because of the json thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to do this one because of the select_credentials
instance method, for the JSON thing we'll have to redefine the data structures at the kt layer (we don't currently need to do this because the inner rust code doesn't yet support it) -- see the Offering.kt
as an example of this
@@ -16,6 +17,13 @@ pub enum RustCoreError { | |||
} | |||
|
|||
impl RustCoreError { | |||
pub fn from_poison_error<T>(error: PoisonError<T>, error_type: &str) -> Arc<Self> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pretty metal name
to: String, | ||
from: String, | ||
exchangeId: String, | ||
data: QuoteData, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we name this quoteData?
val name: String? = null, | ||
val description: String? = null, | ||
val group: String? = null, | ||
val requiredPaymentDetails: JsonNode? = null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having JsonNode here is a bit sus right?
could name these val jsonRequiredPaymentDetails: String
or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope this is right, here's the existing https://github.com/TBD54566975/tbdex-kt/blob/8c6a93b47b1cd0eefdc5910b26c04319e54ad970/protocol/src/main/kotlin/tbdex/sdk/protocol/models/ResourceData.kt#L142
we stringify things over the UniFFI boundary, which is why this (Offering) and the Rfq (both of which have JSON types) are fully written out in kotlin as opposed to using typealias Offering = RustCoreOfferingData
, and then you see below the toRustCore()
and fromRustCore()
which stringify/parse over the UniFFI boundary
import tbdex.sdk.rust.Web5RustCoreException | ||
import tbdex.sdk.web5.* | ||
|
||
class E2ePfiExemplarIntegrationTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is huge! Anyone wanting to see the flow of tbdex I'll send them here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
definitely! right now it has things hardcoded for the given instance of pfi-exemplar, so each dev would have to go in and update the inputs. we can make this better in due time. in rust I split this out into a dedicated executable called pfi_exemplar_integration_test
, so it's not a unit test, it's an app
var count = 0 | ||
while (exchange.close == null) { | ||
Thread.sleep(5000) | ||
exchange = getExchange(pfiDidUri, bearerDid, quote.metadata.exchangeId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we have to spin up a service or something? Is there some delay? why sleeping and retry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah this integration test expects a running instance of the tbdex-pfi-exemplar and developers will have to update the pfiDidUri
, the didURi
and a couple other items for it to work on their end
I think we can make this better so it requires less setup, but that would probably require setting up env vars which are automatically set over in the tbdex-pfi-exemplar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job lots of stuff!
Co-authored-by: nitro-neal <[email protected]>
1af7bdd
to
5d256bf
Compare
Follow-up items