palomachain / paloma

The fast blockchain messenger protocol
Apache License 2.0
291 stars 135 forks source link

chore: change bridge-tax to be per token #1212

Closed maharifu closed 4 months ago

maharifu commented 4 months ago

Related Github tickets

Background

We want bridge taxes to be per token, instead of global.

Testing completed

Breaking changes

maharifu commented 4 months ago

Excellent work.

Blocking this until we're clear on what happens to live data. We have tax data in the keeper already: https://explorer.kjnodes.com/paloma/gov/57

So that means we'll need an upgrade path to ensure the upgrade from 1.15.1 to 1.15.2 won't crash due to type differences.

My suggestion would be to simply clear the store of tax data.

Won't the store be cleared with the rename of gravity to skyway in the 1.15.2 upgrade?

I tested the upgrade in local testnet and it worked fine afterward. I'll re-do the test to make sure I have bridge tax data before the upgrade.

maharifu commented 4 months ago

Of course the store will be cleared anyway. LGTM.

Thanks for checking.

I was just testing locally. The store works fine, but we have an issue with the proposal. Since the proposal changed, we can't decode the previous one anymore.

# palomad q gov proposals
Error: rpc error: code = Unknown desc = runtime error: invalid memory address or nil pointer dereference: panic

I'll look into it.