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

fix wrong behavior: submit multisig txn only there are enough signatures #3482

Merged
merged 7 commits into from
Jun 25, 2022

Conversation

templexxx
Copy link
Contributor

@templexxx templexxx commented Jun 23, 2022

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe): testing

What is the current behavior?

Ignore signatrues' count but submit directly because my mistake.

Issue Number: N/A

What is the new behavior?

  • Checking signatures enough or not before submit.
  • Adding multisig testing: submit txn by local txn signature file

Other information

@templexxx templexxx requested review from nanne007, sanlee42 and nkysg June 23, 2022 15:05
@templexxx templexxx requested a review from jolestar as a code owner June 23, 2022 15:05
@templexxx templexxx marked this pull request as draft June 23, 2022 15:07
@codecov
Copy link

codecov bot commented Jun 23, 2022

Codecov Report

Merging #3482 (7377a35) into master (c656e4b) will decrease coverage by 0.24%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3482      +/-   ##
==========================================
- Coverage   31.29%   31.06%   -0.23%     
==========================================
  Files         500      500              
  Lines       46900    46900              
  Branches    22129    22129              
==========================================
- Hits        14675    14564     -111     
+ Misses      18109    18022      -87     
- Partials    14116    14314     +198     
Flag Coverage Δ
unittests 31.06% <ø> (-0.23%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cmd/starcoin/src/cli_state.rs 48.79% <ø> (-4.87%) ⬇️
rpc/server/src/module/mod.rs 0.00% <0.00%> (-16.66%) ⬇️
...mons/forkable-jellyfish-merkle/src/iterator/mod.rs 41.50% <0.00%> (-10.00%) ⬇️
vm/types/src/on_chain_config/mod.rs 42.47% <0.00%> (-9.58%) ⬇️
chain/open-block/src/lib.rs 31.79% <0.00%> (-9.27%) ⬇️
executor/src/block_executor.rs 24.45% <0.00%> (-8.88%) ⬇️
consensus/src/dummy.rs 65.22% <0.00%> (-8.69%) ⬇️
vm/natives/src/vector.rs 16.67% <0.00%> (-8.33%) ⬇️
vm/types/src/transaction/error.rs 3.58% <0.00%> (-7.14%) ⬇️
chain/src/verifier/mod.rs 35.16% <0.00%> (-7.03%) ⬇️
... and 69 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 c656e4b...7377a35. Read the comment docs.

… execution

1. eprint multisig signatures filepath in txn execution
2. get txn output when there are enough multisig signatures
…esting

1. submit directly
2. sign to file first, then submit
@templexxx templexxx marked this pull request as ready for review June 24, 2022 05:43
@templexxx templexxx requested a review from yourmoonlight as a code owner June 24, 2022 05:43
@templexxx
Copy link
Contributor Author

Need to fix the submit-txn, I wonder how to get value directlly in test like this?

{
  "ok": "/94713c208ff452d4d02c5446eb18f5ab538b72976e08a84cada4b08be68583ab.multisig-txn"
}

@jolestar
Copy link
Member

Need to fix the submit-txn, I wonder how to get value directlly in test like this?

{
  "ok": "/94713c208ff452d4d02c5446eb18f5ab538b72976e08a84cada4b08be68583ab.multisig-txn"
}

use @$ to get the previous command output.

or you can return a struct like:

{
  "ok": {"output_file": "/xxxx"}
}

use @$.output_file to get the response field output_file

@jolestar jolestar linked an issue Jun 24, 2022 that may be closed by this pull request
@templexxx templexxx merged commit 52289a6 into starcoinorg:master Jun 25, 2022
@templexxx templexxx deleted the test.multisig branch June 25, 2022 08:34
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.

[test]增加多签集成测试
2 participants