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.

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

Change default contracts to network version 2 #890

Merged
merged 9 commits into from
Jul 13, 2022

Conversation

awrichar
Copy link
Contributor

@awrichar awrichar commented Jul 7, 2022

Part of FIR-12

Split out separate "pinBatch" and "networkAction" methods in the V2 contract, and make it the default. Adjust the blockchain plugins to handle both versions of the contract cleanly.

Also fix some bugs found along the way with network actions being broken.

@codecov-commenter
Copy link

codecov-commenter commented Jul 7, 2022

Codecov Report

Merging #890 (765a949) into main (074dee5) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 765a949 differs from pull request most recent head 5d81c59. Consider uploading reports for the commit 5d81c59 to get more accurate results

@@           Coverage Diff           @@
##             main     #890   +/-   ##
=======================================
  Coverage   99.96%   99.96%           
=======================================
  Files         298      298           
  Lines       19492    19544   +52     
=======================================
+ Hits        19486    19538   +52     
  Misses          5        5           
  Partials        1        1           
Impacted Files Coverage Δ
internal/blockchain/ethereum/ethereum.go 100.00% <100.00%> (ø)
internal/blockchain/ethereum/eventstream.go 100.00% <100.00%> (ø)
internal/blockchain/fabric/eventstream.go 100.00% <100.00%> (ø)
internal/blockchain/fabric/fabric.go 100.00% <100.00%> (ø)
internal/multiparty/manager.go 100.00% <100.00%> (ø)
internal/multiparty/operations.go 100.00% <100.00%> (ø)
internal/tokens/fftokens/fftokens.go 99.56% <0.00%> (-0.01%) ⬇️
pkg/core/data.go 100.00% <0.00%> (ø)
pkg/core/group.go 100.00% <0.00%> (ø)
pkg/core/message.go 100.00% <0.00%> (ø)
... and 40 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e8f9cc...5d81c59. Read the comment docs.

shorsher
shorsher previously approved these changes Jul 8, 2022
Copy link
Member

@shorsher shorsher left a comment

Choose a reason for hiding this comment

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

👋🏻

@awrichar awrichar marked this pull request as draft July 8, 2022 20:55
@awrichar
Copy link
Contributor Author

awrichar commented Jul 8, 2022

Based on offline discussion, might be worth also splitting the method into separate calls for batch pin / network action (but still both emitting a single shared event). Moving to draft until I can add that change.

awrichar added 8 commits July 8, 2022 19:46
Also rename the "namespace" parameter to "action" in V2, and handle accordingly.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Failure to remove a subscription (ie because it was already removed) should
not cause infinite retries to remove it.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@awrichar awrichar marked this pull request as ready for review July 9, 2022 02:23
@awrichar awrichar requested a review from shorsher July 9, 2022 02:23
@awrichar awrichar dismissed shorsher’s stale review July 9, 2022 02:23

Added significant new changes - needs re-review.

Copy link
Contributor

@jimthematrix jimthematrix left a comment

Choose a reason for hiding this comment

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

solidity and chaincode updates look good to me

@awrichar awrichar merged commit 995d53b into hyperledger:main Jul 13, 2022
@awrichar awrichar deleted the v2 branch July 13, 2022 20:03
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.

4 participants