Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Interfaces v2 #541

Closed
wants to merge 23 commits into from
Closed

Interfaces v2 #541

wants to merge 23 commits into from

Conversation

jaybuidl
Copy link
Member

@jaybuidl jaybuidl commented Jan 27, 2023 •
edited
Loading

Supersedes #511

@netlify
Copy link

netlify bot commented Jan 27, 2023 •
edited
Loading

❌ Deploy Preview for kleros-v2 failed.

Name Link
🔨 Latest commit 3fe7121
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2/deploys/6490fe220f078000084760e9

@jaybuidl jaybuidl changed the base branch from master to feat(web)/demo-staging January 27, 2023 16:14
@jaybuidl jaybuidl self-assigned this Jan 27, 2023
@codeclimate
Copy link

codeclimate bot commented Jan 27, 2023

Code Climate has analyzed commit 52239c3e and detected 65 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 7
Duplication 25
Style 33

View more on Code Climate.

@jaybuidl jaybuidl added Type: Enhancement ✨ Compatibility: ABI change 🗯 Smart contract ABI is changing. Package: Contracts Court smart contracts labels Jan 27, 2023
@jaybuidl jaybuidl force-pushed the feat/interfaces-v2 branch 2 times, most recently from 2754f39 to 5332653 Compare January 27, 2023 16:37
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@jaybuidl jaybuidl mentioned this pull request Jan 27, 2023
Merged
greenlucid
greenlucid reviewed Jan 28, 2023
View reviewed changes
contracts/src/arbitration/IArbitrableV2.sol Outdated
IArbitrableV2 indexed _arbitrator,
uint256 indexed _disputeID,
uint256 _externalDisputeID,
string _disputeContextUri
Copy link

@greenlucid greenlucid Jan 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

In defense of MetaEvidence event (or equivalent DisputeContext), if you emit this separately, and then you reference it with an ID, that's much cheaper.

What should I assume the arbitrator do in this instance, store the _disputeContextUri in storage? Then every challenge will require loading this string from storage (which could be ~6 cold SLOAD or 12k gas when making a challenge).

If you instead were to have the event DisputeContext and you just pass a _disputeContextID to Dispute, then, that's just 1 cold SLOAD.

Base automatically changed from feat(web)/demo-staging to master February 23, 2023 21:42
@jaybuidl jaybuidl changed the base branch from master to dev April 17, 2023 21:24
@jaybuidl jaybuidl force-pushed the feat/interfaces-v2 branch 3 times, most recently from c53b023 to 7b1c2bf Compare April 18, 2023 14:21
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@netlify
Copy link

netlify bot commented Jun 13, 2023 •
edited
Loading

✅ Deploy Preview for kleros-v2-contracts ready!

Name Link
🔨 Latest commit 3fe7121
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2-contracts/deploys/6490fe22cf949c0008bb98d8
😎 Deploy Preview https://deploy-preview-541--kleros-v2-contracts.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@jaybuidl jaybuidl force-pushed the feat/interfaces-v2 branch from 5eb5055 to d3470ad Compare June 13, 2023 19:03
@jaybuidl jaybuidl force-pushed the feat/interfaces-v2 branch from cb66246 to 4078d04 Compare June 14, 2023 20:52
@jaybuidl jaybuidl linked an issue Jun 14, 2023 that may be closed by this pull request
Arbitration interfaces and SDK v2 improvements #938
Open
7 tasks
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 7 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jaybuidl jaybuidl mentioned this pull request Jun 20, 2023
Closed
@jaybuidl jaybuidl mentioned this pull request Jun 23, 2023
Merged
@jaybuidl jaybuidl closed this Jun 23, 2023
@jaybuidl jaybuidl deleted the feat/interfaces-v2 branch June 28, 2023 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greenlucid greenlucid greenlucid left review comments

Assignees

@jaybuidl jaybuidl

Labels
Compatibility: ABI change 🗯 Smart contract ABI is changing. Package: Contracts Court smart contracts Type: Feature🗿
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Arbitration interfaces and SDK v2 improvements
2 participants
@jaybuidl @greenlucid