Skip to content

disperse v2 [WIP]#52

Open
w84april wants to merge 63 commits intodevelopfrom
feat/disperse
Open

disperse v2 [WIP]#52
w84april wants to merge 63 commits intodevelopfrom
feat/disperse

Conversation

@w84april
Copy link
Contributor

@w84april w84april commented Feb 6, 2024

No description provided.

@vercel
Copy link
Contributor

vercel bot commented Feb 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
smoldapp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 2, 2024 0:36am

@w84april w84april changed the title feat: refactor input components disperse v2 Feb 11, 2024
@w84april w84april changed the title disperse v2 disperse v2 [WIP] Feb 11, 2024
const existingEntry = await getEntry({address: entry.address});
if (existingEntry) {
const mergedChains = [...(existingEntry.chains || []), ...(entry.chains || [])];
const mergedChains = [...(entry.chains || [])];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const mergedChains = [...(entry.chains || [])];
const mergedChains = [...(existingEntry.chains || []), ...(entry.chains || [])];

Once you update an entry for a given chain, you want to use the most up-to-date version of it, aka the one in the storage (from existingEntry), not the one in the cache/state (from entry) as base.
So [...storage, ...cache]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This way it's not possible to update the chains completely. It was only possible to add new chains to the entry, but not removing some of them. The reason for that is we were always spreading existingEntry.chains into merged chains which seemed wrong. I think that chains can be the only field to be overwritten. Can you pls provide some example when we actually need to spread the chains like this? It will help to come up with new logic for this

Comment on lines +180 to +182
allSelected.forEach(input => onUpdateStatus(input.UUID, 'pending'));
const {safeTxHash} = await sdk.txs.send({txs: transactions});
allSelected.forEach(input => onUpdateStatus(input.UUID, 'success'));
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if this loops should be merged into one single forof

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants