Feat/stateful arbitrator

[satello]

• Another restructuring of the API. Now the 3 base level directories in src/ are contractWrappers, abstractWrappers and resourceWrappers. Moved to a directory based structure (i.e. abstractWrappers/arbitrator/index.js instead of abstractWrappers/arbitrator.js).

• Arbitrator api now stores the contract instance so that arbitratorAddress never has to be passed. Because of this the functionality is slightly different. Need to pass an arbitrator address when initializing the Kleros api. Or else call kleros.arbitrator.setArbitrator before use of the arbitrator api. We cannot load the contract in the constructor so the address just gets set and then when the first call is made it will check to see if it is loaded and load contractInstance. (There might be an easier way, open to suggestions)

• expose contract api's via kleros.contracts... instead of having pinakion and blockhashrng be initialized root level api's. Only initialized api's are arbitrator and arbitrableContracts

• arbitrableContract -> arbitrableContracts throughout the codebase

[coveralls]

Pull Request Test Coverage Report for Build 574

• 93 of 113 (82.3%) changed or added relevant lines in 14 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+2.6%) to 56.806%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/contractWrappers/arbitrator/StatefulContractWrapper.js	16	17	94.12%
src/contractWrappers/arbitrator/KlerosPOC/index.js	29	33	87.88%
src/resourceWrappers/Notifications/index.js	12	17	70.59%
src/resourceWrappers/Disputes/index.js	8	18	44.44%

Totals
Change from base Build 566:	2.6%
Covered Lines:	554
Relevant Lines:	905

---

💛 - Coveralls

[epiqueras]

Use default params?

[epiqueras]

We don't need getters for public properties.

[satello]

This is needed because at least at the moment abstractWrapper/arbitrator is only inheriting/delegating the functions from KlerosPOC. So there is no way to access variables via the api without a getter

[epiqueras]

You could keep a reference like before, this.contractWrapper.

[satello]

There is one but its private at the moment this._contractWrapper. It kind of defeats the purpose of the delegation if its public, as then all those methods can just be called via arbitrator.contactWrapper...., which maybe isn't a bad alternative to trying to be tricky with the delegation, but does require the consumer to have a deeper knowledge of where their calls live

[satello]

I am going to leave it as is for now

[epiqueras]

StatefulContractWrapper?

[satello]

👍

[epiqueras]

Use named exports instead so we can tree-shake.

[epiqueras]

Next time, don't create releases on a branch. It can cause issues with semver tags when merging after a hotfix or another merge.

[satello]

👍 I am not a huge fan of pushing directly to develop. We can do release branches though

[epiqueras]

Good idea.

[epiqueras]

I don’t think it’s wrong to use it internally, It’s “private usage”.

…

On Thu, Mar 29, 2018 at 3:40 PM Sam Vitello ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/contractWrappers/arbitrator/Arbitrator.js <#114 (comment)>: > + } + } + + /** + * Set a new arbitrator + * @param {string} contractAddress - The address of the contract + * @param {object} artifact - Contract artifact to use to load contract + * @returns {object} contractInstance object + */ + setArbitrator = async (contractAddress, artifact) => { + this.contractAddress = contractAddress || this.contractAddress + this.artifact = artifact || this.artifact + return this._load() + } + + getContractInstance = () => this.contractInstance There is one but its private at the moment this._contractWrapper. It kind of defeats the purpose of the delegation if its public, as then all those methods can just be called via arbitrator.contactWrapper...., which maybe isn't a bad alternative to trying to be tricky with the delegation, but does require the consumer to have a deeper knowledge of where their calls live — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#114 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ASRQaLyykeWsUegGVrgiCsgkbwWM-HMpks5tjWLXgaJpZM4S_leA> .