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

Test bridge pool wasm #737

Merged

Conversation

batconjurer
Copy link
Member

@batconjurer batconjurer commented Nov 4, 2022

This PR depends on #729 and

  • fixes the wasm for submitting a transfer to the ethereum bridge pool.
  • Adds some functionality to a set of testing tools that allow us to test that vps accept changes generated by wasm.
  • Fixes some inconsistencies between erc20 multitoken keys and other multitoken keys (the entirety of which might be addressed later).
  • Adds tests that the wasm changes are accepted by the bridge pool vp.

@batconjurer batconjurer requested review from sug0 and james-chf November 4, 2022 15:00
Copy link
Contributor

@james-chf james-chf left a comment

Choose a reason for hiding this comment

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

some xans need to become nams

shared/src/types/ethereum_events.rs Outdated Show resolved Hide resolved
tests/src/native_vp/eth_bridge_pool.rs Outdated Show resolved Hide resolved
wasm/wasm_source/src/tx_bridge_pool.rs Outdated Show resolved Hide resolved
tests/src/vm_host_env/tx.rs Show resolved Hide resolved
@@ -35,7 +35,7 @@ impl Keys {
self.prefix
.push(&BALANCE_KEY_SEGMENT.to_owned())
.expect("should always be able to construct this key")
.push(&format!("#{}", owner.encode()))
.push(&owner.to_db_key())
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah this way is much better, the previous format way was a hack

&albert_address(),
&xan(),
None,
BERTHA_WEALTH.into(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

is albert stealing from bertha?

Copy link
Member Author

Choose a reason for hiding this comment

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

That bastard!

wasm/wasm_source/src/tx_bridge_pool.rs Show resolved Hide resolved
@sug0 sug0 self-requested a review November 11, 2022 11:20
Copy link
Contributor

@james-chf james-chf left a comment

Choose a reason for hiding this comment

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

lgtm, there are a few things to PR back to main:

  • TestTxEnv::execute_tx fn
  • the namadac tx flag changes
  • TestTxEnv::credit_tokens signature change and usages in wasm/wasm_source

tests/src/e2e/ledger_tests.rs Show resolved Hide resolved
tests/src/vm_host_env/tx.rs Show resolved Hide resolved
@batconjurer
Copy link
Member Author

lgtm, there are a few things to PR back to main:

* `TestTxEnv::execute_tx` fn

* the `namadac tx` flag changes

* `TestTxEnv::credit_tokens` signature change and usages in `wasm/wasm_source`

Covered by PRs #738 and #775

@batconjurer
Copy link
Member Author

pls update wasm

@batconjurer batconjurer merged commit fdd211a into eth-bridge-integration Nov 14, 2022
@batconjurer batconjurer deleted the bat/ethbridge/test-bridge-pool-wasms branch November 14, 2022 10:52
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.

3 participants