Skip to content
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

BugFix: Termed Payments #2479

Open
Da-Colon opened this issue Nov 13, 2024 · 5 comments
Open

BugFix: Termed Payments #2479

Da-Colon opened this issue Nov 13, 2024 · 5 comments
Assignees

Comments

@Da-Colon
Copy link
Contributor

Da-Colon commented Nov 13, 2024

So uncovered an issue with tying termed roles to the payments.

Currently when we retrieve regular role payments, we do this via the recipient, its smart account address, which is unique to the role.

For termed roles its the nominee who is the recipient. So what does this mean.

Currently when retrieving the payments, I'm creating a set of recipients created from all the terms nominees of a hat. The problem that it will now pull ANY Sablier payments of that address. This includes those created in other termed hats that nominee has worn and streams that happen outside of our UI.

@Da-Colon
Copy link
Contributor Author

Da-Colon commented Nov 13, 2024

I can think of two solutions. Ones feels convoluted and one involves updating the contracts....again..

Option 1

Make payment start date time more unique by setting the Term's EndDate's time to the current time of the user's current date.

Then we can filter out any payments that out tied to that particular EndDateTs, since the changes of two payment end date + time being the same will be unlikely.

Option 2

Probably the better one. is we just tie the stream to the hat/term via the KeyValuePairs contract and update the contract and interface accordingly.

Thoughts @decentdao/engineering?

@Da-Colon Da-Colon self-assigned this Nov 13, 2024
@adamgall
Copy link
Member

Option 3

Get streams based on creator (safe), not recipient.

@Da-Colon
Copy link
Contributor Author

Da-Colon commented Nov 13, 2024

That would solve the outside Decent, Sablier problem. hmmm Then we would tie the payment to term via termEndDateTs and nominee of a particular role.

that could work. I think it still leaves open the possibility of streams showing up on incorrect roles, since we still really aren't tying it to the HatId in anyway and currently the TermEndDateTs, is set to the beginning of the Date selected. maybe I need to think more. probably unlikely.

@Da-Colon
Copy link
Contributor Author

Da-Colon commented Nov 13, 2024

is set to the beginning of the Date selected

Saw I originally put that in the code for the DatePicker but maybe needs more thought on which to choose endOfDay or beginning of Day or current time of day

@Da-Colon
Copy link
Contributor Author

I'll be working on getting this straighten out tonight when I get back to it.

So I think my plan is to

  • Update the DatePicker, when a Date is chosen the time is set to the 'now' time (for minutes, etc) versus end or beginning of the day. This will make the termEndDateTs harder for payments to be similar

  • Update the way we pull in streams for termed roles. Using the by creator path I'll get all streams payments made by the safe. then finding matching term+nominees and assigned those payments to that hat. For regular roles will still grab streams using smartAddress

@decentdao/engineering any final thoughts leave here for me tonight before I go to do this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In progress
Development

No branches or pull requests

2 participants