kleros / kleros-api-DEPRECATED

A Javascript library that makes it easy to build relayers and other DApps that use the Kleros protocol. DEPRECATED use https://github.com/kleros/archon for interfacing with standard arbitration contracts.
MIT License
21 stars 15 forks source link

Race condition with arbitrable contracts #138

Closed satello closed 5 years ago

satello commented 6 years ago

An unforeseen consequence of the design choice to load and store the contract instance in the ContractImplementation class and to apply it to both arbitrable and arbitrator contracts is that we have race conditions between notifications or other background tasks and user actions.

For example: When we refresh the page while loading a contract in kleros.tech there is a race condition between notifications which is setting the arbitrable instance and the user who is trying to load a different arbitrable contract to fetch data.

epiqueras commented 6 years ago

Notifications should use their own instance.

satello commented 6 years ago

Yeah that's what I did. We need to further separate disputes and notifications however. There is still going to be a race condition between kleros.disputes.getDisputeData and notifications since Disputes also has to be passed the "internal" instance of arbitrable contract because it registers some notification callbacks

satello commented 6 years ago

I think some of the bulkiness and problematic listeners will get sorted out when the store cutdown is reflected in the api. This will eliminate the timestamp event callbacks which is what the majority of the dispute event listeners are for. What do you think about having each notification handler spin up its own instance of arbitrable contract? It's bulky. But using them as throwaway classes would completely fix the problem

epiqueras commented 6 years ago

Then we can have two or more “internal” instances. Let’s try to get away with the least amount possible while still having no race conditions. On Wed, May 30, 2018 at 2:48 PM Sam Vitello notifications@github.com wrote:

I think some of the bulkiness and problematic listeners will get sorted out when the store cutdown is reflected in the api. This will eliminate the timestamp event callbacks which is what the majority of the dispute event listeners are for. What do you think about having each notification handler spin up its own instance of arbitrable contract? It's bulky. But using them as throwaway classes would completely fix the problem

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kleros/kleros-api/issues/138#issuecomment-393294318, or mute the thread https://github.com/notifications/unsubscribe-auth/ASRQaFBHKXXwOqK2t5CI_JkqWMkrWpIWks5t3vemgaJpZM4UT25m .