Diana/fix auxin registration #69
Conversation
DianaNites
force-pushed
the
diana/fix-auxin-registration
branch
from
719550b to
c76dde9
Compare
|
might not be quite ready yet.. @technillogue can you test this when you get a chance? |
|
Something with the recipients store is borked still, bots will need to message a user first in order to have a working session. Probably need to look into how signal-cli handles this. But registration seems to be working well. |
cxloe
requested review from
NotGyro and
itdaniher
and removed request for
NotGyro
| } | ||
| None => { | ||
| store.last_id += 1; | ||
| store.recipients.push(Recipient::new(store.last_id, self)); |
There was a problem hiding this comment.
Are there any consequences to transferring ownership here?
| serde_json::error::Category::Data | ||
| | serde_json::error::Category::Syntax | ||
| | serde_json::error::Category::Eof => RecipientsStore::new(self), | ||
| _ => Err(e)?, |
There was a problem hiding this comment.
Maybe import the errors directly @ the top of the file to cleanup the code?
| .send() | ||
| .await?; | ||
| // TODO(Diana): We don't currently care about the response | ||
| let _profile: auxin::profile::ProfileResponse = res.json().await?; |
There was a problem hiding this comment.
| let _profile: auxin::profile::ProfileResponse = res.json().await?; | |
| let _: auxin::profile::ProfileResponse = res.json().await?; |
There was a problem hiding this comment.
Does the result need to be driven by an await even? If it is, maybe it can be launched as a task instead of awaiting immediately and the handle can be awaited later when it's absolutely necessary to do so.
| &mut rand::thread_rng(), | ||
| )?; | ||
| client | ||
| .put("https://chat.signal.org/v1/profile") |
There was a problem hiding this comment.
These signal endpoint uris are moving targets - so Maybe we can have some sort of config enum that contains the endpoints data instead of hardcoding it.
There was a problem hiding this comment.
Yes, that was to-do list item #1 when I was planning to refactor Auxin. It really needs to be defined declarative somewhere rather than just having this string literal all over the place. Otherwise we can't use Signal testnet.
That might be out of the scope of this PR though.
| .await?; | ||
|
|
||
| // TODO(Diana): This endpoint seems to be hit, | ||
| // but I don't think signal-cli saves its result? |
There was a problem hiding this comment.
What does this endpoint achieve for Auxin?
There was a problem hiding this comment.
Appears to be session magic within Signal's server. There's a good bit of that and none of it is documented. Try removing it and see what breaks.
|
getting this fixed and merged would be great on account of mobilecoinofficial/forest#220 |
No description provided.