-
Notifications
You must be signed in to change notification settings - Fork 180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Sign/Web3Wallet] Add pending proposals #744
Conversation
guard let proposal = try? record.request.params?.get(SessionType.ProposeParams.self) | ||
else { return nil } | ||
|
||
return Session.Proposal( |
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.
you can use SessionProposal's publicRepresentation(pairingTopic: String) -> Session.Proposal
here
let requestSubscriptionPayloads = pendingHistory | ||
.compactMap { record -> RequestSubscriptionPayload<SessionType.ProposeParams>? in | ||
guard let proposalParams = mapProposeParams(record) else { | ||
return nil | ||
} | ||
return RequestSubscriptionPayload(id: record.id, topic: record.topic, request: proposalParams, publishedAt: Date()) | ||
} | ||
|
||
requestSubscriptionPayloads.forEach { | ||
let proposal = $0.request | ||
proposalPayloadsStore.set($0, forKey: proposal.proposer.publicKey) | ||
} |
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.
you do it to set values to proposalPayloadsStore on app launch?
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.
make sense
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.
Only when you request for pending proposals
guard let proposalParams = mapProposeParams(record) else { | ||
return nil | ||
} | ||
return RequestSubscriptionPayload(id: record.id, topic: record.topic, request: proposalParams, publishedAt: Date()) |
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.
I don't like we are recreating RequestSubscriptionPayload type here.
could we change type stored by proposalPayloadsStore?
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.
Me neither. Your proposal sounds legit 👍
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.
in general looks good!
|
||
requestSubscriptionPayloads.forEach { | ||
let proposal = $0.request | ||
proposalPayloadsStore.set($0, forKey: proposal.proposer.publicKey) |
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.
why we cannot remove proposalPayloadsStore and depend on rpc history only?
If big array of expired proposals the only reason I think it's fine. We can fix it when implement expiration logic for proposals
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.
oh yeah, good catch
@alexander-lsvk any blockers on this PR? |
Due Dilligence