Conversation
2da98ff to
58ea608
Compare
58ea608 to
77013e3
Compare
1e51e3b to
1484d9e
Compare
cdfba92 to
7457c2f
Compare
|
Looking good! Can we also get tests for:
|
7457c2f to
7dd5024
Compare
| requests: Bag, | ||
| pending_withdrawals: Bag, | ||
| /// Tracks pending withdrawal IDs for iteration (Bag doesn't support iteration). | ||
| pending_withdrawal_ids: vector<address>, |
There was a problem hiding this comment.
This may warant changing the pending_withdrawal_ids data structure to something like a LinkedTable.
There was a problem hiding this comment.
So i'm not a huge fan of this because it requires unbounded iteration that must terminate inside of a single sui transaction. These types of things can lead to deadlock situations where a txn can never terminate because it takes too much gas or loads too many objects. imo i don't think we need this, or the ability to reassign indexes in the end_reconfig function and instead should introduce another function that can be used to re-key an explicitly provided set of ids. Given the epoch is encoded in there we can iterate through these offchain and selectively bump them before we go to signing them.
Thoughts? @manolisliolios can keep me honest here too
There was a problem hiding this comment.
Good point. puts a lot of load on the end of epoch tx as well as possible deadlock on that critical tx. I think a new function for reassigning explicit withdrawals makes the most sense. We will need to guard this new function with a certificate like we do for each stage of the withdrawal flow (ex. confirm_withdrawal)
There was a problem hiding this comment.
My original approach was lazy reassignment at signing time. I turned to this approach because the original approach requires a fair amount of plumbing (new RPC endpoint, and leader logic to fan out, aggregate, submit), and there was some urgency to unblock devnet. But it may be no longer urgent to merge this PR now as the issue it tries to fix is very unlikely to happen (see my elaboration in chat), and I can turn back to the original approach. WDYT?
#193