Skip to content

feat(SessionKeyRegistry.sol): add deposit-n-rebate#9

Open
DarkLord017 wants to merge 1 commit into
FilOzone:mainfrom
DarkLord017:feat/#7
Open

feat(SessionKeyRegistry.sol): add deposit-n-rebate#9
DarkLord017 wants to merge 1 commit into
FilOzone:mainfrom
DarkLord017:feat/#7

Conversation

@DarkLord017

Copy link
Copy Markdown

Will solve #7

  • Introduced deposit requirements for each permission slot (SLOT_DEPOSIT) and added logic to enforce minimum and maximum expiry times for permissions in SessionKeyRegistry.sol. Deposits are now tracked per permission using the new PermissionData struct.

  • Added new functions: clearExpired to remove expired permissions and refund deposits, and withdrawRefund to allow users to withdraw refundable deposits. Refund logic is now handled via the refundAmount mapping and emits events for deposit addition and refunds.

  • I'm still unsure if that's what was expected like I'm not sure about like how to decide on time limit on expiry and how much should a user deposit

  • Also should i also add max permission length to prevent out-of-gas issues

  • I have used the same mapping to include deposit amount also to reduce more storage

  • Also how should we handle duplication like currently it just adds that deposited amount also if expiry time is higher and rebates all the amount when the expiry passed is it correct ?

If my implmentation looks fine i will start working on tests

Pls review @wjmelements

@rvagg rvagg left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has to be on hold for now, it's not in scope for GA so would rather not merge it until we're ready to start making changes in here

@wjmelements

Copy link
Copy Markdown
Collaborator

this has to be on hold for now, it's not in scope for GA so would rather not merge it until we're ready to start making changes in here

I disagree because the GA contracts are already deployed and frozen. This repo is not an archive so it is fine to enhance it.

@rvagg

rvagg commented Dec 10, 2025

Copy link
Copy Markdown

Well, I'd rather we not be making our lives harder by adding confusion just yet and iterating on the contracts where we don't plan on deploying those iterations pre-GA. Yes git tags solve for this, but let's avoid footguns where we can and keep things simple. I don't think we should have encouraged this work at this stage.

@BigLep BigLep added this to FOC May 22, 2026
@github-project-automation github-project-automation Bot moved this to 📌 Triage in FOC May 22, 2026
@BigLep BigLep moved this from 📌 Triage to 🐱 Todo in FOC May 22, 2026
@github-project-automation github-project-automation Bot moved this from 🐱 Todo to ⌨️ In Progress in FOC May 22, 2026
@BigLep BigLep moved this from ⌨️ In Progress to 🐱 Todo in FOC Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🐱 Todo

Development

Successfully merging this pull request may close these issues.

4 participants